mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(title): prevent stale background title generation from reloading unloaded Ollama models
Add a runtime_validator callback to generate_title() / auto_title_session() / maybe_auto_title(). Callers snapshot the session's model+provider when spawning the background titler; the validator runs right before the LLM request and skips it silently when the live runtime no longer matches — so a stale title request can't reload a model that strict_single_load already evicted after a user model switch. Fail-open: a raising validator never disables titling. Wired at all four call sites (cli, gateway, tui_gateway, acp_adapter). Surgical reapply of PR #19137 (base was 8k+ commits stale; the original patch predates the pinned-language prompts, the atomic-write helper, and the moved TUI/ACP call sites). Original work by @Thatgfsj. Closes #19027.
This commit is contained in:
parent
61be8b3112
commit
ef1c622105
7 changed files with 135 additions and 1 deletions
|
|
@ -1617,6 +1617,11 @@ class HermesACPAgent(acp.Agent):
|
|||
self._send_session_info_update(session_id),
|
||||
)
|
||||
|
||||
# Snapshot the runtime identity; the validator lets the
|
||||
# background titler skip its LLM call if the session's model
|
||||
# changed before it fires (#19027).
|
||||
_title_model = getattr(state.agent, "model", None)
|
||||
_title_provider = getattr(state.agent, "provider", None)
|
||||
maybe_auto_title(
|
||||
self.session_manager._get_db(),
|
||||
session_id,
|
||||
|
|
@ -1630,6 +1635,10 @@ class HermesACPAgent(acp.Agent):
|
|||
"api_key": getattr(state.agent, "api_key", None),
|
||||
"api_mode": getattr(state.agent, "api_mode", None),
|
||||
},
|
||||
runtime_validator=lambda: (
|
||||
getattr(state.agent, "model", None) == _title_model
|
||||
and getattr(state.agent, "provider", None) == _title_provider
|
||||
),
|
||||
title_callback=_notify_title_update,
|
||||
)
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -19,6 +19,12 @@ logger = logging.getLogger(__name__)
|
|||
FailureCallback = Callable[[str, BaseException], None]
|
||||
TitleCallback = Callable[[str], None]
|
||||
|
||||
# Validation callback: () -> bool. Called right before the LLM request in
|
||||
# generate_title(). Return False to skip — e.g. the user switched models
|
||||
# after this background thread captured its runtime snapshot, and sending
|
||||
# the request would reload a model the runtime already evicted (#19027).
|
||||
RuntimeValidator = Callable[[], bool]
|
||||
|
||||
_TITLE_PROMPT = (
|
||||
"Generate a short, descriptive title (3-7 words) for a conversation that starts with the "
|
||||
"following exchange. The title should capture the main topic or intent. "
|
||||
|
|
@ -71,6 +77,7 @@ def generate_title(
|
|||
timeout: Optional[float] = None,
|
||||
failure_callback: Optional[FailureCallback] = None,
|
||||
main_runtime: dict = None,
|
||||
runtime_validator: Optional[RuntimeValidator] = None,
|
||||
) -> Optional[str]:
|
||||
"""Generate a session title from the first exchange.
|
||||
|
||||
|
|
@ -82,11 +89,26 @@ def generate_title(
|
|||
auxiliary call raises — the caller typically wires this to
|
||||
``AIAgent._emit_auxiliary_failure`` so the user sees a warning instead
|
||||
of silently accumulating untitled sessions.
|
||||
|
||||
``runtime_validator`` is called right before the LLM request. If it
|
||||
returns False (e.g. the user's model was switched since the background
|
||||
thread captured its runtime snapshot), the call is skipped silently —
|
||||
no request is sent, so a stale title request can't reload a model the
|
||||
runtime already unloaded (#19027).
|
||||
"""
|
||||
if not _auto_title_enabled():
|
||||
logger.debug("Auto-title skipped: auxiliary.title_generation.enabled=false")
|
||||
return None
|
||||
|
||||
if runtime_validator is not None:
|
||||
try:
|
||||
if not runtime_validator():
|
||||
logger.debug("Title generation skipped: runtime validator returned False")
|
||||
return None
|
||||
except Exception:
|
||||
# Fail open: a broken validator must not disable titling.
|
||||
logger.debug("Title runtime validator raised; proceeding", exc_info=True)
|
||||
|
||||
# Truncate long messages to keep the request small
|
||||
user_snippet = user_message[:500] if user_message else ""
|
||||
assistant_snippet = assistant_response[:500] if assistant_response else ""
|
||||
|
|
@ -193,6 +215,7 @@ def auto_title_session(
|
|||
failure_callback: Optional[FailureCallback] = None,
|
||||
main_runtime: dict = None,
|
||||
title_callback: Optional[TitleCallback] = None,
|
||||
runtime_validator: Optional[RuntimeValidator] = None,
|
||||
) -> None:
|
||||
"""Generate and set a session title if one doesn't already exist.
|
||||
|
||||
|
|
@ -201,6 +224,7 @@ def auto_title_session(
|
|||
- session_db is None
|
||||
- session already has a title (user-set or previously auto-generated)
|
||||
- title generation fails
|
||||
- runtime_validator returns False (model was switched)
|
||||
|
||||
Never lets an exception escape: this is a daemon-thread target, and an
|
||||
escaping exception would spray a raw traceback into the user's terminal
|
||||
|
|
@ -220,6 +244,7 @@ def auto_title_session(
|
|||
failure_callback=failure_callback,
|
||||
main_runtime=main_runtime,
|
||||
title_callback=title_callback,
|
||||
runtime_validator=runtime_validator,
|
||||
)
|
||||
except Exception as e:
|
||||
# WARNING (not debug) so operators see it in agent.log; the message
|
||||
|
|
@ -245,6 +270,7 @@ def _auto_title_session(
|
|||
failure_callback: Optional[FailureCallback] = None,
|
||||
main_runtime: dict = None,
|
||||
title_callback: Optional[TitleCallback] = None,
|
||||
runtime_validator: Optional[RuntimeValidator] = None,
|
||||
) -> None:
|
||||
"""Body of :func:`auto_title_session` — see its docstring."""
|
||||
if not session_db or not session_id:
|
||||
|
|
@ -278,7 +304,11 @@ def _auto_title_session(
|
|||
set_accounting_context(session_db, session_id)
|
||||
|
||||
title = generate_title(
|
||||
user_message, assistant_response, failure_callback=failure_callback, main_runtime=main_runtime
|
||||
user_message,
|
||||
assistant_response,
|
||||
failure_callback=failure_callback,
|
||||
main_runtime=main_runtime,
|
||||
runtime_validator=runtime_validator,
|
||||
)
|
||||
if not title:
|
||||
return
|
||||
|
|
@ -306,6 +336,7 @@ def maybe_auto_title(
|
|||
failure_callback: Optional[FailureCallback] = None,
|
||||
main_runtime: dict = None,
|
||||
title_callback: Optional[TitleCallback] = None,
|
||||
runtime_validator: Optional[RuntimeValidator] = None,
|
||||
) -> None:
|
||||
"""Fire-and-forget title generation after the first exchange.
|
||||
|
||||
|
|
@ -337,6 +368,7 @@ def maybe_auto_title(
|
|||
"failure_callback": failure_callback,
|
||||
"main_runtime": main_runtime,
|
||||
"title_callback": title_callback,
|
||||
"runtime_validator": runtime_validator,
|
||||
},
|
||||
daemon=True,
|
||||
name="auto-title",
|
||||
|
|
|
|||
10
cli.py
10
cli.py
|
|
@ -12686,6 +12686,12 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
_title_failure_cb = getattr(
|
||||
self.agent, "_emit_auxiliary_failure", None
|
||||
) if self.agent else None
|
||||
# Snapshot the runtime identity; the validator lets the
|
||||
# background titler skip its LLM call if the user switches
|
||||
# models before it fires (a stale request would reload an
|
||||
# unloaded Ollama model, #19027).
|
||||
_title_model = self.model
|
||||
_title_provider = self.provider
|
||||
maybe_auto_title(
|
||||
self._session_db,
|
||||
self.session_id,
|
||||
|
|
@ -12700,6 +12706,10 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
"api_key": self.api_key,
|
||||
"api_mode": self.api_mode,
|
||||
},
|
||||
runtime_validator=lambda: (
|
||||
getattr(self, "model", None) == _title_model
|
||||
and getattr(self, "provider", None) == _title_provider
|
||||
),
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
|
|
|||
|
|
@ -19869,6 +19869,12 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
"Gateway auto-title failure suppressed (not user-visible): %s: %s",
|
||||
task, exc,
|
||||
)
|
||||
# Snapshot the runtime identity; the validator lets the
|
||||
# background titler skip its LLM call if the session's
|
||||
# model changed before it fires (a stale request would
|
||||
# reload an unloaded Ollama model, #19027).
|
||||
_title_model = getattr(agent, "model", None) if agent else None
|
||||
_title_provider = getattr(agent, "provider", None) if agent else None
|
||||
maybe_auto_title_kwargs = {
|
||||
"failure_callback": _title_failure_cb,
|
||||
"main_runtime": {
|
||||
|
|
@ -19878,6 +19884,10 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
"api_key": getattr(agent, "api_key", None),
|
||||
"api_mode": getattr(agent, "api_mode", None),
|
||||
} if agent else None,
|
||||
"runtime_validator": (lambda: (
|
||||
getattr(agent, "model", None) == _title_model
|
||||
and getattr(agent, "provider", None) == _title_provider
|
||||
)) if agent else None,
|
||||
}
|
||||
if self._is_telegram_topic_lane(source):
|
||||
maybe_auto_title_kwargs["title_callback"] = lambda title: self._schedule_telegram_topic_title_rename(
|
||||
|
|
|
|||
|
|
@ -369,6 +369,7 @@ AUTHOR_MAP = {
|
|||
"dirtyren@users.noreply.github.com": "dirtyren",
|
||||
"s96919@gmail.com": "s96919",
|
||||
"rasitakyol@hotmail.com": "rasitakyol",
|
||||
"thatgfsj@gmail.com": "Thatgfsj",
|
||||
"141703117+seagpt@users.noreply.github.com": "seagpt",
|
||||
"yakimenkoleksander228@gmail.com": "doxe0x",
|
||||
"a54983334@163.com": "Code-suphub",
|
||||
|
|
|
|||
|
|
@ -387,6 +387,7 @@ class TestMaybeAutoTitle:
|
|||
failure_callback=None,
|
||||
main_runtime=None,
|
||||
title_callback=None,
|
||||
runtime_validator=None,
|
||||
)
|
||||
|
||||
def test_skips_when_title_generation_disabled(self):
|
||||
|
|
@ -430,6 +431,7 @@ class TestMaybeAutoTitle:
|
|||
failure_callback=_cb,
|
||||
main_runtime=None,
|
||||
title_callback=None,
|
||||
runtime_validator=None,
|
||||
)
|
||||
|
||||
def test_skips_if_no_response(self):
|
||||
|
|
@ -513,3 +515,64 @@ class TestAutoTitleDuplicateHandling:
|
|||
db.set_session_title.return_value = False
|
||||
with pytest.raises(RuntimeError):
|
||||
_persist_session_title(db, "missing", "Some Title")
|
||||
|
||||
|
||||
class TestRuntimeValidator:
|
||||
"""runtime_validator gating (#19027): a stale background title request
|
||||
must not fire when the session's model/provider changed after spawn."""
|
||||
|
||||
def test_skips_when_validator_returns_false(self):
|
||||
with patch("agent.title_generator.call_llm") as mock_llm:
|
||||
title = generate_title(
|
||||
"question", "answer",
|
||||
runtime_validator=lambda: False,
|
||||
)
|
||||
assert title is None
|
||||
mock_llm.assert_not_called()
|
||||
|
||||
def test_allows_when_validator_returns_true(self):
|
||||
mock_response = MagicMock()
|
||||
mock_response.choices = [MagicMock()]
|
||||
mock_response.choices[0].message.content = "Validated Title"
|
||||
|
||||
with patch("agent.title_generator.call_llm", return_value=mock_response) as mock_llm:
|
||||
title = generate_title(
|
||||
"question", "answer",
|
||||
runtime_validator=lambda: True,
|
||||
)
|
||||
assert title == "Validated Title"
|
||||
mock_llm.assert_called_once()
|
||||
|
||||
def test_broken_validator_fails_open(self):
|
||||
mock_response = MagicMock()
|
||||
mock_response.choices = [MagicMock()]
|
||||
mock_response.choices[0].message.content = "Resilient Title"
|
||||
|
||||
def _bad_validator():
|
||||
raise RuntimeError("validator gone")
|
||||
|
||||
with patch("agent.title_generator.call_llm", return_value=mock_response) as mock_llm:
|
||||
title = generate_title(
|
||||
"question", "answer",
|
||||
runtime_validator=_bad_validator,
|
||||
)
|
||||
assert title == "Resilient Title"
|
||||
mock_llm.assert_called_once()
|
||||
|
||||
def test_forwards_runtime_validator_to_worker(self):
|
||||
db = MagicMock()
|
||||
db.get_session_title.return_value = None
|
||||
history = [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "assistant", "content": "hi there"},
|
||||
]
|
||||
|
||||
def _v():
|
||||
return True
|
||||
|
||||
with patch("agent.title_generator.auto_title_session") as mock_auto:
|
||||
maybe_auto_title(db, "sess-1", "hello", "hi there", history, runtime_validator=_v)
|
||||
import time
|
||||
time.sleep(0.3)
|
||||
kwargs = mock_auto.call_args.kwargs
|
||||
assert kwargs["runtime_validator"] is _v
|
||||
|
|
|
|||
|
|
@ -9678,6 +9678,11 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None:
|
|||
from agent.title_generator import maybe_auto_title
|
||||
|
||||
_title_key = session.get("session_key") or sid
|
||||
# Snapshot the runtime identity; the validator lets the
|
||||
# background titler skip its LLM call if the session's
|
||||
# model changed before it fires (#19027).
|
||||
_title_model = getattr(agent, "model", None)
|
||||
_title_provider = getattr(agent, "provider", None)
|
||||
maybe_auto_title(
|
||||
_get_db(),
|
||||
_title_key,
|
||||
|
|
@ -9695,6 +9700,10 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None:
|
|||
"api_key": getattr(agent, "api_key", None),
|
||||
"api_mode": getattr(agent, "api_mode", None),
|
||||
},
|
||||
runtime_validator=lambda: (
|
||||
getattr(agent, "model", None) == _title_model
|
||||
and getattr(agent, "provider", None) == _title_provider
|
||||
),
|
||||
# Push the generated title live so the sidebar renames
|
||||
# without waiting for the next list refresh (the titler
|
||||
# runs async, after this turn's refresh already fired).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue