diff --git a/acp_adapter/server.py b/acp_adapter/server.py index 7fc0a7403e474..d558d289dbd28 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -1907,7 +1907,7 @@ class HermesACPAgent(acp.Agent): # Auto-titling fires inside the turn prologue now; give the agent # this session's notifier so a new title reaches the client as a # session-info update instead of waiting for the next one. - def _notify_title_update(_title: str) -> None: + def _notify_title_update(_title: str, _source: str) -> None: if conn: loop.call_soon_threadsafe( asyncio.create_task, diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 5b38c0ab82a63..6ead8eff6aadf 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -735,39 +735,86 @@ _FAST_MODEL_FAMILIES: tuple = ( # opposite of what a titler wants; ":batch" is an async queue, not a live # endpoint; embedding models ("all-minilm") match "-mini" but aren't chat # models at all; ":free" tiers are heavily rate-limited and measured slowest. +# The modality suffixes are the same trap as the embedders — a provider names +# its speech and image endpoints after the chat model they're paired with, so +# "gpt-4o-mini-tts" satisfies the "-mini" rung and cannot answer a prompt. _FAST_MODEL_EXCLUDE: tuple = ( "thinking", "reason", "-r1", "minilm", ":batch", ":free", "o1-", "o3-", "o4-", "codex", "audio", "-vl", "embed", + "-tts", "-transcribe", "-realtime", "-image", "-search-preview", ) +_VERSION_CHUNK_RE = re.compile(r"(\d+(?:\.\d+)?)") + + +def _model_recency_key(model_id: str) -> tuple: + """Sort key that puts a family's newest release first (descending). + + The rungs at the bottom of ``_FAST_MODEL_FAMILIES`` are bare family names — + ``-mini``, ``-flash``, ``haiku`` — and a provider serves every generation of + those it hasn't retired. Compared as plain strings, the oldest wins: + ``gpt-3.5-mini`` sorts before ``gpt-5.4-mini``, and ``claude-3-haiku`` before + ``claude-haiku-4.5``. So the rung meant to keep us current on a provider's + small tier was pinning us to its most obsolete member. + + Splitting digit runs out and comparing them as numbers fixes both the + generation order and the 9-vs-10 cliff a string sort walks off. + """ + chunks = [] + for index, part in enumerate(_VERSION_CHUNK_RE.split(model_id.lower())): + if not part: + continue + # re.split with one capturing group alternates text, number, text, … + chunks.append((1, float(part), "") if index % 2 else (0, 0.0, part)) + return tuple(chunks) + + def _fast_model_from_catalog(provider_id: str) -> str: """Pick the fastest small model the provider ACTUALLY serves right now. Reads the provider's live (cached) ``/v1/models`` catalog and returns the - first ``_FAST_MODEL_FAMILIES`` match. Returns "" when the catalog is + newest ``_FAST_MODEL_FAMILIES`` match. Returns "" when the catalog is unavailable or holds no small model, so the caller falls through to the provider's curated default. Never raises and never blocks on a cold network path — the underlying fetch is memory+disk cached with a last-known-good fallback. """ try: + from hermes_cli.auth import resolve_api_key_provider_credentials from hermes_cli.models import fetch_models_with_pricing from providers import get_provider_profile - profile = get_provider_profile(provider_id) - base_url = str(getattr(profile, "base_url", "") or "").rstrip("/") + # The provider's own credentials, because most ``/v1/models`` endpoints + # are authenticated: fetched anonymously they 401, and the caller reads + # that as "this provider serves no small model" and quietly falls back + # to the curated default forever. + api_key, base_url = "", "" + try: + creds = resolve_api_key_provider_credentials(provider_id) or {} + api_key = str(creds.get("api_key", "")).strip() + base_url = str(creds.get("base_url", "")).strip() + except Exception: + # Not an API-key provider, or nothing configured yet. The anonymous + # fetch below still works for the catalogs that allow it. + logger.debug("No credentials for %s catalog", provider_id, exc_info=True) + + if not base_url: + base_url = str(getattr(get_provider_profile(provider_id), "base_url", "") or "") + base_url = base_url.rstrip("/") if not base_url: return "" # fetch_models_with_pricing appends its own /v1/models. if base_url.endswith("/v1"): base_url = base_url[:-3] - catalog = fetch_models_with_pricing(base_url=base_url, timeout=3.0) or {} + catalog = fetch_models_with_pricing( + api_key=api_key or None, base_url=base_url, timeout=3.0 + ) or {} except Exception: logger.debug("Fast-model catalog lookup failed for %s", provider_id, exc_info=True) return "" - ids = sorted(str(m) for m in catalog) + ids = sorted((str(m) for m in catalog), key=_model_recency_key, reverse=True) for family in _FAST_MODEL_FAMILIES: for model_id in ids: lowered = model_id.lower() diff --git a/agent/title_generator.py b/agent/title_generator.py index 74ecfdb701e68..cb8b662fa442e 100644 --- a/agent/title_generator.py +++ b/agent/title_generator.py @@ -21,9 +21,11 @@ import json import logging import re import threading -from typing import Callable, Optional +from typing import Any, Callable, Optional from agent.auxiliary_client import call_llm +from agent.context_compressor import LEGACY_SUMMARY_PREFIX +from agent.message_content import flatten_message_text logger = logging.getLogger(__name__) @@ -32,7 +34,18 @@ logger = logging.getLogger(__name__) # so silent-drops (e.g. OpenRouter 402 exhausting the fallback chain) # become visible instead of piling up as NULL session titles. FailureCallback = Callable[[str, BaseException], None] -TitleCallback = Callable[[str], None] + +# Callback signature: (title, source) -> None, where source is the provenance +# the title was persisted under (``derived`` for the instant slice of the user's +# own words, ``llm`` for the model's upgrade of it). +# +# Titling is two-stage, and the stage matters to the consumer. A local surface +# wants both, so the sidebar renames instantly and sharpens a second later. A +# consumer that spends a rate-limited remote call per title — renaming a Discord +# thread, a Telegram topic — wants ``llm`` only: acting on both burns two calls +# to end up at the same name, and on Discord (2 renames per 10 minutes per +# channel) the throwaway one can be what survives. +TitleCallback = Callable[[str, str], None] # Validation callback: () -> bool. Called right before the LLM request in # generate_title(). Return False to skip — e.g. the user switched models @@ -111,11 +124,24 @@ _CONTROL_WRAPPERS = ( ) # Hermes' own machine-authored openers. A compaction handoff or a resumed -# session must not be titled after the scaffolding that carried it. +# session must not be titled after the scaffolding that carried it. The legacy +# summary prefix comes from the compressor rather than a fourth local copy — +# compaction still emits it, and a session named after it is named after us. _MACHINE_PREFIXES = ( "[CONTEXT COMPACTION", + LEGACY_SUMMARY_PREFIX, "[Runtime note:", + "[System note:", "[SYSTEM]", + # Model-switch marker from tui_gateway.server._append_model_switch_marker. + # It is persisted with role="user" (strict OpenAI-compatible providers + # reject a system message that is not first — #48338), so without this + # entry it looks like a real opening turn: switching models before the + # first real message titled the session + # "[System: The active model for this chat has…" instead of the user's + # actual question. Keep in sync with + # tui_gateway.server._MODEL_SWITCH_MARKER_PREFIX. + "[System: The active model for this chat has changed to ", ) @@ -392,7 +418,7 @@ def generate_title( return None -def _persist_session_title(session_db, session_id, title, *, source): +def _persist_session_title(session_db, session_id, title, *, source, dedupe=True): """Persist a title at *source* authority, recovering from name collisions. The write goes through ``set_auto_title`` (precedence check + write in one @@ -401,6 +427,14 @@ def _persist_session_title(session_db, session_id, title, *, source): session (the unique-title index); rather than leave the session untitled (#50537), append a ``#N`` suffix via ``get_next_title_in_lineage``. + ``dedupe=False`` re-raises that collision instead. The derived title is the + one write on the turn's critical path, and it is also the one that collides + constantly — it is a slice of the user's own words, and people open sessions + with "hi" and "help me debug this". Scanning the lineage for the next free + "hi #N" is a widening scan, run inline, for a name the model replaces a + second later. The background stage picks the collision back up, so nothing + is lost by declining it here. + Returns the title actually persisted, or None when a higher-authority title already held the row (nothing was written). """ @@ -429,7 +463,7 @@ def _persist_session_title(session_db, session_id, title, *, source): return _set(title) except ValueError: next_title_fn = getattr(session_db, "get_next_title_in_lineage", None) - if next_title_fn is None: + if not dedupe or next_title_fn is None: raise deduped = next_title_fn(title) if not deduped or deduped == title: @@ -458,11 +492,11 @@ def apply_instant_title( if not title: return None persisted = _persist_session_title( - session_db, session_id, title, source="derived" + session_db, session_id, title, source="derived", dedupe=False ) if persisted and title_callback is not None: try: - title_callback(persisted) + title_callback(persisted, "derived") except Exception: logger.debug("Instant-title callback failed", exc_info=True) return persisted @@ -573,25 +607,76 @@ def _auto_title_session( main_runtime=main_runtime, runtime_validator=runtime_validator, ) + source = "llm" if not title: - return + # No model title, so the derived one has to hold — and it may never have + # been written, since the inline attempt declines a name collision + # rather than scan the lineage on the turn's critical path. Off that + # path the scan is affordable, so spend it here and leave the session + # named rather than nameless. + title = derive_title(user_message) + source = "derived" + if not title: + return try: - persisted = _persist_session_title( - session_db, session_id, title, source="llm" - ) + persisted = _persist_session_title(session_db, session_id, title, source=source) if persisted is None: return logger.debug("Auto-generated session title: %s", persisted) if title_callback is not None: try: - title_callback(persisted) + title_callback(persisted, source) except Exception: logger.debug("Auto-title callback failed", exc_info=True) except Exception as e: logger.debug("Failed to set auto-generated title: %s", e) +def _is_real_user_turn(message: Any) -> bool: + """Whether a history entry is a question a person actually asked. + + Hermes persists a lot of machinery under ``role="user"`` — compaction + handoffs, model-switch markers, background-process notices — because strict + OpenAI-compatible providers reject a system message that isn't first. + Counting those as turns is what made a session that merely *opened* with one + look like it was already past the point where titling applies. + + A multimodal turn is judged on its text, so "here's a screenshot, fix the + login" counts as the real question it is. + """ + if not isinstance(message, dict) or message.get("role") != "user": + return False + content = message.get("content") + + return is_titleable_user_message( + content if isinstance(content, str) else flatten_message_text(content) + ) + + +def _session_is_untitled(session_db, session_id: str) -> bool: + """Whether the session still carries no title of any provenance. + + Titling normally reads the opening message and nothing else, but an opener + isn't always titleable: an image with no caption, a compaction handoff, a + bare slash command. Those sessions stayed nameless for life — the same guard + that stops us re-titling on every turn also stopped us ever trying again. + This reopens the question on later turns, and only while the answer is still + missing, so a named session asks nothing and pays nothing. + + Answers False when it can't tell: an unreadable title is not a reason to + start spending a model call per turn. + """ + getter = getattr(session_db, "get_session_title", None) + if not callable(getter): + return False + try: + return not str(getter(session_id) or "").strip() + except Exception: + logger.debug("Untitled check failed for %s", session_id, exc_info=True) + return False + + def maybe_auto_title( session_db, session_id: str, @@ -614,17 +699,18 @@ def maybe_auto_title( if not session_db or not session_id or not user_message: return - # Count user messages to detect the opening turn. ``conversation_history`` - # is the state BEFORE this turn's message is appended when called from the - # turn prologue, and after it when called post-response, so accept both. - # Entries are dicts; anything else means a caller passed the wrong - # positional and titling must degrade quietly rather than raise. - user_msg_count = sum( - 1 - for m in (conversation_history or []) - if isinstance(m, dict) and m.get("role") == "user" - ) - if user_msg_count > 1: + # Count the real questions behind us to detect the opening turn. + # ``conversation_history`` is the state BEFORE this turn's message is + # appended when called from the turn prologue, and after it when called + # post-response, so accept both. + # + # Two things have to be true to skip: we are past the opening turn AND the + # session already has a name. Either alone gets it wrong. The count alone + # left a session that opened with machinery permanently nameless, because + # nothing reconsidered it. The title alone would never title at all on a + # store too old to report one. + 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): return if not is_titleable_user_message(user_message): diff --git a/agent/turn_context.py b/agent/turn_context.py index ab514c14aa94f..7c36a923aabf8 100644 --- a/agent/turn_context.py +++ b/agent/turn_context.py @@ -170,18 +170,36 @@ def append_notes_to_multimodal_content(content: Any, notes: str) -> bool: return False +# Surfaces whose sessions must not be auto-titled. The prologue is shared by +# EVERY agent, not only the ones a human is watching, so membership here is what +# keeps the titler off machine-driven runs: +# +# - cron — the scheduler names its own session after the job in its `finally` +# block, and the opener is the cron delivery hint, not a user's request. +# Titling it writes that scaffolding as the visible name for the whole run and +# bills a side-LLM call per fire, against the same job that sets +# `skip_memory` / `skip_background_review` to avoid exactly that. +# - subagent — a delegated child's session is hidden from every picker, so its +# title is never read. A batch at `max_concurrent_children` would pay N title +# calls for N names nobody sees. +_UNTITLED_PLATFORMS = frozenset({"cron", "subagent"}) + + def _maybe_title_session_at_turn_start(agent: Any, messages: List[Any]) -> None: """Kick off auto-titling for this session's first user message. - Called from the turn prologue, so every surface (CLI, gateway, TUI/desktop, - ACP) gets identical behavior without each one re-implementing the call. - Fully defensive: titling is cosmetic and must never break a turn. + Called from the turn prologue, so every surface a human reads (CLI, gateway, + TUI/desktop, ACP) gets identical behavior without each one re-implementing + the call. Fully defensive: titling is cosmetic and must never break a turn. """ session_db = getattr(agent, "_session_db", None) session_id = getattr(agent, "session_id", None) if not session_db or not session_id: return + if str(getattr(agent, "platform", "") or "").lower() in _UNTITLED_PLATFORMS: + return + try: from agent.message_content import flatten_message_text from agent.title_generator import maybe_auto_title diff --git a/apps/desktop/src/app/chat/index.tsx b/apps/desktop/src/app/chat/index.tsx index f8c2c86a6790c..1b01516f23e73 100644 --- a/apps/desktop/src/app/chat/index.tsx +++ b/apps/desktop/src/app/chat/index.tsx @@ -20,7 +20,7 @@ import { TitleMenuTrigger } from '@/components/ui/title-menu-trigger' import { type HermesGateway } from '@/hermes' import { useI18n } from '@/i18n' import type { ChatMessage } from '@/lib/chat-messages' -import { quickModelOptions, sessionTitle } from '@/lib/chat-runtime' +import { NEW_SESSION_TITLE, quickModelOptions, sessionTitle } from '@/lib/chat-runtime' import { useIncrementalExternalStoreRuntime } from '@/lib/incremental-external-store-runtime' import { modelOptionsQueryKey, requestModelOptions } from '@/lib/model-options' import { cn } from '@/lib/utils' @@ -116,7 +116,7 @@ function ChatHeader({ const activeStoredSession = (selectedSessionId && sessions.find(session => sessionMatchesStoredId(session, selectedSessionId))) || null - const title = activeStoredSession ? sessionTitle(activeStoredSession) : 'New session' + const title = activeStoredSession ? sessionTitle(activeStoredSession) : NEW_SESSION_TITLE // Which agent/persona owns this chat — glanceable in the header once a // second profile exists, so the open session's ownership is never ambiguous diff --git a/apps/desktop/src/app/chat/pane-mirror.ts b/apps/desktop/src/app/chat/pane-mirror.ts index 03c7e5bc3cc88..05800c110bdcd 100644 --- a/apps/desktop/src/app/chat/pane-mirror.ts +++ b/apps/desktop/src/app/chat/pane-mirror.ts @@ -36,6 +36,10 @@ export interface PaneMirror { * self-subscribing component (e.g. a session's status dot) so the strip needn't * re-sync on status/color change — only `title` drives re-registration. */ tabLead?: (key: string) => ReactNode + /** Custom label NODE for the tile's tab, self-subscribing for the same reason + * as `tabLead` — a name that moves faster than re-registration (see + * PaneChrome.tabTitle). Falls back to `title`. */ + tabTitle?: (key: string) => ReactNode /** Glyph buttons the tile contributes to the strip, after the last tab (where * "+" sits), while it is the ACTIVE pane — e.g. a preview's console / * DevTools toggles. DATA, not markup: the strip's `PaneStripGlyph` owns the @@ -82,6 +86,7 @@ export function paneMirror(cfg: PaneMirror): () => void { title, data: { tabLead: cfg.tabLead ? () => cfg.tabLead!(key) : undefined, + tabTitle: cfg.tabTitle ? () => cfg.tabTitle!(key) : undefined, stripTools: cfg.stripTools ? () => cfg.stripTools!(key) : undefined, dock: { before: cfg.before?.(tile), diff --git a/apps/desktop/src/app/chat/session-draft-title.tsx b/apps/desktop/src/app/chat/session-draft-title.tsx new file mode 100644 index 0000000000000..e4e9a665c2887 --- /dev/null +++ b/apps/desktop/src/app/chat/session-draft-title.tsx @@ -0,0 +1,25 @@ +import { NEW_SESSION_TITLE } from '@/lib/chat-runtime' +import { useStoreSelector } from '@/lib/use-session-slice' +import { $draftTitles, draftTitleIn } from '@/store/composer' + +export interface SessionDraftTitleProps { + /** The draft's composer key — a tile's stored session id, or null for the + * new chat that has no session yet. */ + scope: null | string +} + +/** + * A DRAFT'S NAME — what an unsent session is called until it has a real one. + * + * The tab of a session that has never been sent renders this instead of its + * registered title, because the name moves with the composer: every debounced + * stash republishes it. Re-registering the contribution at that rate would + * re-render the whole panes area, so the label subscribes for itself and its + * own key only. + * + * Falls back to the placeholder rather than going blank, so an emptied composer + * reads the same as one never typed into. + */ +export function SessionDraftTitle({ scope }: SessionDraftTitleProps) { + return useStoreSelector($draftTitles, titles => draftTitleIn(titles, scope)) || NEW_SESSION_TITLE +} diff --git a/apps/desktop/src/app/chat/session-status-dot.tsx b/apps/desktop/src/app/chat/session-status-dot.tsx index 1f457c10154dd..ab808866a8b91 100644 --- a/apps/desktop/src/app/chat/session-status-dot.tsx +++ b/apps/desktop/src/app/chat/session-status-dot.tsx @@ -66,6 +66,15 @@ const DOT_VARIANTS: Record = { role: 'status', title: r => r.finishedUnread }, + // Hollow grey, the faintest ink the app has — nothing has ever run here. It + // shares the outline with `background` because both mean "open, not + // producing", and sits a shade dimmer because a draft is the one state that + // has yet to do anything at all. + draft: { + ariaLabel: r => r.draftSession, + className: `${DOT_BASE} border border-(--ui-text-quaternary)`, + title: r => r.draftSession + }, // Settled: the project color, or nothing at all. An uncolored session used to // get a grey dot, which put a mark of the same weight as a status next to // every resting row and made "no color" look like a state of its own. @@ -78,8 +87,11 @@ export interface SessionStatusDotProps { /** The STORED session id — the key every live-state atom (working / * attention / stalled / unread / background) is keyed by, on BOTH surfaces: * the sidebar row's `session.id` and a pane tile's `storedSessionId` are the - * same stored id (`$workingSessionIds` et al. map `storedSessionId`). */ - storedSessionId: string + * same stored id (`$workingSessionIds` et al. map `storedSessionId`). + * + * Null on a new chat that has yet to reach the backend — no id to key by, + * and no turn behind it, which is the draft state by definition. */ + storedSessionId: null | string /** The session row for color resolution — recents OR the project tree. Both * call sites already hold it; passing it lets the idle dot inherit the * project color even for a session older than the paginated recents page @@ -112,7 +124,9 @@ export function SessionStatusDot({ storedSessionId, session, branchStem, classNa // Selector, not a plain useStore: the map is rebuilt whenever any session's // status changes, but a given dot only repaints when ITS OWN state flips. - const dotState = useStoreSelector($sessionDotStateById, states => states[storedSessionId] ?? 'idle') + const dotState = useStoreSelector($sessionDotStateById, states => + storedSessionId ? (states[storedSessionId] ?? 'idle') : 'draft' + ) const variant = DOT_VARIANTS[dotState] return ( diff --git a/apps/desktop/src/app/chat/session-tile.tsx b/apps/desktop/src/app/chat/session-tile.tsx index 9c30a632f9796..e362a64d35db7 100644 --- a/apps/desktop/src/app/chat/session-tile.tsx +++ b/apps/desktop/src/app/chat/session-tile.tsx @@ -33,8 +33,8 @@ import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { transcribeAudio } from '@/hermes' import { useI18n } from '@/i18n' import type { ChatMessage } from '@/lib/chat-messages' -import { sessionTitle } from '@/lib/chat-runtime' -import { createComposerAttachmentScope } from '@/store/composer' +import { NEW_SESSION_TITLE, sessionTitle } from '@/lib/chat-runtime' +import { createComposerAttachmentScope, draftTitleFor } from '@/store/composer' import { $pinnedSessionIds, pinSession, unpinSession } from '@/store/layout' import { $activeGatewayProfile } from '@/store/profile' import { $projectTree } from '@/store/projects' @@ -61,6 +61,7 @@ import type { SessionDragPayload } from './composer/inline-refs' import { type ComposerScope, ComposerScopeProvider } from './composer/scope' import { useComposerActions } from './hooks/use-composer-actions' import { paneMirror } from './pane-mirror' +import { SessionDraftTitle } from './session-draft-title' import { startSessionDrag } from './session-drag' import { SessionStatusDot } from './session-status-dot' import { useSessionTileActions } from './session-tile-actions' @@ -374,19 +375,24 @@ export function tileStoredRow(storedSessionId: string): SessionInfo | undefined ) } +/** The tab's REGISTERED name. Deliberately the bare placeholder for a draft + * rather than its live composer title (`tabTitle` renders that): re-registering + * per keystroke would re-render the strip, and holding the draft's text here + * would let the registered name already match the row that lands on send — + * skipping the re-register that hands the tab back to this string. */ function tileTitle(storedSessionId: string): string { const stored = tileStoredRow(storedSessionId) - // A tab-strip "+" tab is unlisted until its first turn persists, so it isn't - // in $sessions yet — label it "New session" rather than a bare "Session". - return stored ? sessionTitle(stored) : 'New session' + return stored ? sessionTitle(stored) : NEW_SESSION_TITLE } -/** The `@session` link payload for a tile tab drag — id + owning profile + title. */ +/** The `@session` link payload for a tile tab drag — id + owning profile + title. + * Resolved at drag time, so an unsent tab drags under its draft name. */ function tileDragPayload(storedSessionId: string): SessionDragPayload { const stored = tileStoredRow(storedSessionId) + const title = stored ? sessionTitle(stored) : draftTitleFor(storedSessionId) || NEW_SESSION_TITLE - return { id: storedSessionId, profile: stored?.profile ?? '', title: tileTitle(storedSessionId) } + return { id: storedSessionId, profile: stored?.profile ?? '', title } } // --------------------------------------------------------------------------- @@ -588,6 +594,10 @@ export const watchSessionTiles = paneMirror({ tabLead: storedSessionId => ( ), + // Until the first turn lists a row there is no title to register, so the tab + // takes its name from the composer instead — live, without re-registering. + tabTitle: storedSessionId => + tileStoredRow(storedSessionId) ? null : , render: storedSessionId => , tabWrap: (storedSessionId, tab) => ( { registry.register({ id: 'workspace', area: 'panes', - title: stored ? storedSessionTitle(stored) : 'New session', + // The placeholder, not the draft's live name — `tabTitle` below renders + // that. Keeping it here would re-register the pane on every keystroke. + title: stored ? storedSessionTitle(stored) : NEW_SESSION_TITLE, data: { // The tab's status dot — the SAME primitive the sidebar row and session - // tiles render, so the main tab never disagrees with its sidebar row. No - // dot on a fresh draft (no session yet). - tabLead: selected ? () => : undefined, + // tiles render, so the main tab never disagrees with its sidebar row. A + // fresh draft has no session to key by, which IS its status: the dot + // resolves to `draft` and marks the tab rather than leaving a hole. + tabLead: () => , + // A draft's name lives in its composer, not in any session row, so the + // label subscribes to it directly — typing renames the tab without + // re-registering the pane. + tabTitle: stored ? undefined : () => , // Pages aren't tab-able: the main zone's bar stands down while one shows. headerVeto: $workspaceIsPage.get(), placement: 'main', diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts b/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts index 46b49ec514dc4..f70b8222ec755 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts +++ b/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts @@ -81,6 +81,12 @@ interface PaneChrome extends PaneSizing { * the tab and the sidebar row render status/color from the ONE primitive * (self-subscribing — it updates without the strip re-registering). */ tabLead?: () => React.ReactNode + /** This pane's TAB LABEL, when it changes faster than the contribution + * should. A session pane whose draft is being typed renames on every + * debounce beat; re-registering `title` that often would re-render the + * whole panes area, so the label subscribes for itself instead. Absent, or + * returning nothing, falls back to `title`. */ + tabTitle?: () => React.ReactNode /** Glyph buttons this pane contributes to the strip, rendered after the last * tab (where "+" sits) while the pane is ACTIVE — controls that act on the * pane, not on any one tab: a preview's console / DevTools toggles. DATA, not diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx index d22dd513e666f..312685b53df52 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx @@ -313,6 +313,10 @@ export function TreeGroup({ // leaving the tree. const closeableTab = (paneId: string) => !paneChrome(paneFor(paneId)).uncloseable || panesWithCloser.has(paneId) + // A pane's own live label when it has one, else its registered string. + const tabLabel = (paneId: string) => + paneChrome(paneFor(paneId)).tabTitle?.() ?? paneFor(paneId)?.title ?? paneId + // Collapse/restore a tool panel (or plain minimize elsewhere) — the header // chevron + tap gesture, routed so ⌃`/the titlebar toggle stay truthful. const toggleCollapse = () => (node.minimized ? restoreTreePane(activeId) : collapseTreePane(activeId)) @@ -379,7 +383,6 @@ export function TreeGroup({ > {shown.map(paneId => { const closeable = closeableTab(paneId) - const title = paneFor(paneId)?.title ?? paneId return ( - {title} + {tabLabel(paneId)} ) })} @@ -548,7 +551,7 @@ export function TreeGroup({ {chrome.tabLead ? ( {chrome.tabLead()} ) : null} - {title} + {tabLabel(paneId)} ) diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 9a6853609d06c..e2663ecef881e 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -1644,6 +1644,7 @@ export const ar = defineLocale({ needsInput: 'تحتاج إدخالا', waitingForAnswer: 'بانتظار إجابة', backgroundRunning: 'تعمل في الخلفية', + draftSession: 'مسودة — لم تُرسل بعد', finishedUnread: 'اكتملت وفيها جديد', hideTabBar: 'إخفاء شريط التبويبات', openInNewTab: 'فتح في تبويب جديد', diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 3c5ce76723559..cb3ac391fedb6 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -1966,6 +1966,7 @@ export const en: Translations = { waitingForAnswer: 'Waiting for your answer', finishedUnread: 'Finished — unread', backgroundRunning: 'Background task running', + draftSession: 'Draft — nothing sent yet', handoffOrigin: platform => `Handed off from ${platform}`, ownedByProfile: profile => `Profile: ${profile}`, renamed: 'Renamed', diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 1d8ebb907e00f..8742c3f0dce8b 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -1784,6 +1784,7 @@ export const ja = defineLocale({ waitingForAnswer: '回答を待っています', finishedUnread: '完了 — 未読', backgroundRunning: 'バックグラウンドタスク実行中', + draftSession: '下書き — 未送信', handoffOrigin: platform => `${platform} から引き継ぎ`, ownedByProfile: profile => `プロファイル: ${profile}`, renamed: '名前を変更しました', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index c68958353600f..6a33c3631a52b 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1658,6 +1658,7 @@ export interface Translations { waitingForAnswer: string finishedUnread: string backgroundRunning: string + draftSession: string handoffOrigin: (platform: string) => string ownedByProfile: (profile: string) => string renamed: string diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index b614fe8abbfd8..977eacb4e4077 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -1726,6 +1726,7 @@ export const zhHant = defineLocale({ waitingForAnswer: '等待您的回答', finishedUnread: '已完成 — 未讀', backgroundRunning: '背景任務執行中', + draftSession: '草稿 — 尚未傳送', handoffOrigin: platform => `從 ${platform} 轉接`, ownedByProfile: profile => `設定檔:${profile}`, renamed: '已重新命名', diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 92b7bf5cd109c..45c1b7bb7aa8c 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -2158,6 +2158,7 @@ export const zh: Translations = { waitingForAnswer: '正在等待你的回答', finishedUnread: '已完成 — 未读', backgroundRunning: '后台任务运行中', + draftSession: '草稿 — 尚未发送', handoffOrigin: platform => `从 ${platform} 转接`, ownedByProfile: profile => `配置档:${profile}`, renamed: '已重命名', diff --git a/apps/desktop/src/lib/chat-runtime.ts b/apps/desktop/src/lib/chat-runtime.ts index c581daaea9c7e..1aa826eec39b6 100644 --- a/apps/desktop/src/lib/chat-runtime.ts +++ b/apps/desktop/src/lib/chat-runtime.ts @@ -66,6 +66,10 @@ export function sessionTitle(session: SessionInfo): string { return session.title?.trim() || session.preview?.trim() || 'Untitled session' } +/** What a session is called before it has been sent — and before its composer + * has been typed into, which is the only thing that can name it earlier. */ +export const NEW_SESSION_TITLE = 'New session' + export function coerceGatewayText(value: unknown): string { if (typeof value === 'string') { return value diff --git a/apps/desktop/src/lib/draft-title.ts b/apps/desktop/src/lib/draft-title.ts new file mode 100644 index 0000000000000..2c980b147296d --- /dev/null +++ b/apps/desktop/src/lib/draft-title.ts @@ -0,0 +1,40 @@ +import { SLASH_COMMAND_RE } from './chat-runtime' + +/** Matches `agent/title_generator.py`'s MAX_DERIVED_TITLE_CHARS, so a draft + * doesn't visibly reflow the moment the backend's derived title replaces it. */ +const MAX_DRAFT_TITLE_CHARS = 48 + +/** + * Name a draft after what the user has typed into it. + * + * The client-side twin of the backend's `derive_title`: first meaningful line, + * whitespace collapsed, cut on a word boundary. It runs before any session + * exists, so it can't reach the real titler — a draft has no persisted row and + * no opening message yet, which is exactly what `apply_instant_title` needs. + * + * Empty when there's nothing worth naming, so the caller keeps "New session" + * rather than showing a title that says less than the placeholder. + */ +export function deriveDraftTitle(text: string): string { + const line = text.split('\n').find(candidate => candidate.trim())?.trim() ?? '' + + if (!line) { + return '' + } + + // A bare `/skin` names the draft after the command rather than the work, the + // failure the backend titler summarizes away. Title from the argument instead; + // with no argument there is no intent yet, so the placeholder stands. + const body = (SLASH_COMMAND_RE.test(line) ? line.replace(/^\/\S+\s*/, '') : line).split(/\s+/).join(' ') + + if (body.length <= MAX_DRAFT_TITLE_CHARS) { + return body + } + + // Cut on a word boundary, unless that would throw away more than half of it. + const cut = body.slice(0, MAX_DRAFT_TITLE_CHARS) + const space = cut.lastIndexOf(' ') + const kept = space > MAX_DRAFT_TITLE_CHARS / 2 ? cut.slice(0, space) : cut + + return `${kept.replace(/[\s,.;:—-]+$/, '')}…` +} diff --git a/apps/desktop/src/store/composer.ts b/apps/desktop/src/store/composer.ts index 6783db9637abd..9f2d541f9b9aa 100644 --- a/apps/desktop/src/store/composer.ts +++ b/apps/desktop/src/store/composer.ts @@ -1,5 +1,6 @@ import { atom } from 'nanostores' +import { deriveDraftTitle } from '@/lib/draft-title' import { triggerHaptic } from '@/lib/haptics' export interface ComposerAttachment { @@ -151,6 +152,50 @@ function loadPersistedDraftTexts(): [string, SessionDraft][] { const draftsBySession = new Map(loadPersistedDraftTexts()) +/** + * What each unsent draft would be called, keyed the same way its text is. + * + * A draft has no session to carry a title, so the tab showing it reads this + * instead of the "New session" placeholder. Written from `stashSessionDraft`, + * the one funnel every composer's text already flows through — the debounce + * that persists a draft is the same beat that renames its tab, so typing costs + * nothing extra. Only tabs showing a draft subscribe, and each selects its own + * key, so a rename repaints one label rather than the strip. + * + * Seeded from the persisted texts: a draft left open across a restart comes + * back already named. + */ +export const $draftTitles = atom>( + Object.fromEntries( + [...draftsBySession].map(([key, draft]) => [key, deriveDraftTitle(draft.text)]).filter(([, title]) => title) + ) +) + +/** Read one draft's title out of the map — for a `useStoreSelector`, so a tab + * repaints on its OWN rename rather than on every draft's. */ +export const draftTitleIn = (titles: Record, scope: string | null | undefined): string => + titles[draftKey(scope)] ?? '' + +export const draftTitleFor = (scope: string | null | undefined): string => draftTitleIn($draftTitles.get(), scope) + +function publishDraftTitle(key: string, title: string): void { + const current = $draftTitles.get() + + if ((current[key] ?? '') === title) { + return + } + + const next = { ...current } + + if (title) { + next[key] = title + } else { + delete next[key] + } + + $draftTitles.set(next) +} + /** * Re-read the persisted drafts written by ANOTHER window into this one's map. * @@ -169,12 +214,14 @@ export function reloadPersistedDrafts(): void { for (const [key, draft] of incoming) { const local = draftsBySession.get(key) draftsBySession.set(key, local?.attachments.length ? { ...local, text: draft.text } : draft) + publishDraftTitle(key, deriveDraftTitle(draft.text)) } // A key that vanished from storage was cleared (sent) in the other window. for (const key of [...draftsBySession.keys()]) { if (!incoming.has(key)) { draftsBySession.delete(key) + publishDraftTitle(key, '') } } } @@ -257,6 +304,7 @@ export function stashSessionDraft(scope: string | null | undefined, text: string } persistDraftTexts() + publishDraftTitle(key, deriveDraftTitle(text)) } export function takeSessionDraft(scope: string | null | undefined): SessionDraft { diff --git a/apps/desktop/src/store/session-dot-state.ts b/apps/desktop/src/store/session-dot-state.ts index 3a27e41172171..2d8a56a0f7575 100644 --- a/apps/desktop/src/store/session-dot-state.ts +++ b/apps/desktop/src/store/session-dot-state.ts @@ -23,9 +23,9 @@ import { stableRecord } from '@/lib/stable-array' import { $backgroundRunningSessionIds } from './composer-status' import { $sessions, $unreadFinishedSessionIds, lineageAliases } from './session' -import { $attentionSessionIds, $stalledSessionIds, $workingSessionIds } from './session-states' +import { $attentionSessionIds, $draftSessionIds, $stalledSessionIds, $workingSessionIds } from './session-states' -export type SessionDotState = 'background' | 'idle' | 'needs-input' | 'stalled' | 'unread' | 'working' +export type SessionDotState = 'background' | 'draft' | 'idle' | 'needs-input' | 'stalled' | 'unread' | 'working' /** The sidebar row's arc. A quiet turn is still authoritatively running, so * `stalled` keeps it; a blocking prompt drops it, because the amber dot is the @@ -46,9 +46,10 @@ export const $sessionDotStateById = computed( $stalledSessionIds, $backgroundRunningSessionIds, $unreadFinishedSessionIds, + $draftSessionIds, $sessions ], - (attention, working, stalled, background, unread, sessions) => { + (attention, working, stalled, background, unread, draft, sessions) => { const next: Record = {} const claim = (ids: readonly string[], state: SessionDotState) => { @@ -62,6 +63,10 @@ export const $sessionDotStateById = computed( // Weakest claim first — each pass overwrites the one above it, so the order // below IS the priority order. A blocking prompt outranks everything: it is // the only state that needs the user. + // + // Draft is weakest of all: it says only "no turn has happened here yet", so + // the first thing that does happen speaks over it. + claim(draft, 'draft') claim(unread, 'unread') claim(background, 'background') claim(working, 'working') diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index d127d6bcfc583..79020f0450bbd 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -39,6 +39,7 @@ import { $sessions, $unreadFinishedSessionIds, lineageAliases, + sessionMatchesStoredId, setActiveSessionStoredIdRotation } from './session' import { isSecondaryWindow } from './windows' @@ -308,6 +309,38 @@ export const $attentionSessionIds = computed( )) ) +// An open session nothing has ever been sent to — the ⌘T tab whose backend +// session exists but is unlisted, or a tile still waiting on its first send. +// `blankDraftTile`'s predicate, read as a status rather than as a slot to spend. +// +// The row's own `message_count` is the tiebreaker, and it is load-bearing: a +// session RESUMING also holds an empty message list for the moment between +// binding its runtime and loading its transcript, and calling that a draft +// would flash the wrong mark on a conversation with years of history in it. +let draftIds: readonly string[] = [] +export const $draftSessionIds = computed([$sessionStates, $sessions], (states, sessions) => { + const unsent = (state: ClientSessionState) => { + if (state.busy || state.messages.length > 0) { + return false + } + + const storedId = state.storedSessionId + + // No stored id is the ⌘T tab that hasn't reached the backend yet: a draft + // by definition, and no row to consult. Asking anyway would match a row on + // an empty lineage root. + if (!storedId) { + return true + } + + const row = sessions.find(session => sessionMatchesStoredId(session, storedId)) + + return !row || row.message_count === 0 + } + + return (draftIds = stableArray(draftIds, storedIds(states, sessions, unsent))) +}) + // --------------------------------------------------------------------------- // Session tiles. // --------------------------------------------------------------------------- diff --git a/contributors/emails/yy28@vip.sina.com b/contributors/emails/yy28@vip.sina.com new file mode 100644 index 0000000000000..373e5eb78060e --- /dev/null +++ b/contributors/emails/yy28@vip.sina.com @@ -0,0 +1 @@ +yy28 diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 73f7cf2941028..9e071864294c6 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -85,6 +85,10 @@ class RelayAdapter(BasePlatformAdapter): # feedback off SendResult — see send()). Consumed by the gateway's # semantic thread-rename lane; bounded like the sibling caches. self._auto_thread_by_chat: Dict[str, Tuple[str, str]] = {} + # chat_id -> event fired when the entry above lands, so a consumer that + # arrives before the send can wait for it instead of polling. See + # wait_for_auto_thread_info. + self._auto_thread_waiters: Dict[str, asyncio.Event] = {} # chat_id -> chat_type (e.g. "dm", "channel", "group") learned from the # inbound event. Used to reproduce native Slack's synthetic-DM-thread # suppression on the relay lane: a DM streaming reply carries @@ -993,6 +997,13 @@ class RelayAdapter(BasePlatformAdapter): ) except Exception: # noqa: BLE001 - feedback capture must never break send pass + # Wake the rename lane on EVERY send into this chat, not only the ones + # that auto-threaded. It is waiting to learn where this turn's reply + # landed, and "nowhere new" is an answer — one it should get now rather + # than by outlasting a timeout. + waiter = self._auto_thread_waiters.get(str(chat_id)) + if waiter is not None: + waiter.set() return SendResult( success=bool(result.get("success")), message_id=result.get("message_id"), @@ -1007,6 +1018,42 @@ class RelayAdapter(BasePlatformAdapter): gateway's semantic thread-rename lane (auto session title).""" return self._auto_thread_by_chat.get(str(chat_id)) + async def wait_for_auto_thread_info( + self, chat_id: str, timeout: float + ) -> Optional[Tuple[str, str]]: + """``auto_thread_info_for_chat``, but willing to wait for the send. + + The rename lane asks where the reply landed as soon as the session is + titled, and the session is titled from the user's opening message — + before the model has answered, let alone before we've sent anything. So + the question arrives a whole turn early, and a turn is a one-liner or + twenty minutes of tool calls. + + Waits for the next send into this chat and then answers, so a reply the + connector didn't auto-thread reports its miss as soon as it's sent + instead of holding until *timeout* — which is only a backstop for a turn + that never sends at all. + """ + info = self.auto_thread_info_for_chat(chat_id) + if info is not None: + return info + key = str(chat_id) + waiter = self._auto_thread_waiters.get(key) + if waiter is None: + waiter = asyncio.Event() + self._auto_thread_waiters[key] = waiter + try: + await asyncio.wait_for(waiter.wait(), timeout) + except asyncio.TimeoutError: + return None + finally: + # Only the waiter we may have installed, and only if no later call + # replaced it; a fired event must not be left behind to make the + # next turn's wait return instantly on stale feedback. + if self._auto_thread_waiters.get(key) is waiter: + self._auto_thread_waiters.pop(key, None) + return self.auto_thread_info_for_chat(chat_id) + def _resolve_reply_to_for_send( self, chat_id: str, diff --git a/gateway/run.py b/gateway/run.py index c3aae82cf53af..52df713f7f40c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1016,6 +1016,17 @@ def _startup_restore_drain_timeout_secs() -> float: return float(_STARTUP_RESTORE_DRAIN_TIMEOUT_SECS_DEFAULT) +def _as_thread_info(info: Any) -> Optional[Tuple[str, str]]: + """*info* as a (thread_id, initial_name) pair, or None if it isn't one. + + The pair comes back across the relay connector boundary, so its shape is + the connector's word rather than ours. + """ + if isinstance(info, tuple) and len(info) == 2 and all(isinstance(x, str) for x in info): + return cast(Tuple[str, str], info) + return None + + def _float_env(name: str, default: float) -> float: """Read an env var as float, falling back to ``default`` on typos/empty. @@ -4460,9 +4471,15 @@ class TurnRunner: session_id = getattr(agent, "session_id", None) source = ctx.source + # Both lanes below spend a rate-limited platform call per title, so + # they take the model's title and skip the derived one — see + # TitleCallback. Renaming twice lands on the same name at twice the + # cost, and Discord's 2-per-10-minutes channel budget can spend + # itself on the throwaway and drop the one worth showing. if self._runner._is_telegram_topic_lane(source): - agent._on_session_title = lambda title: ( - self._runner._schedule_telegram_topic_title_rename( + agent._on_session_title = lambda title, title_source: ( + title_source == "llm" + and self._runner._schedule_telegram_topic_title_rename( source, session_id, title, ) ) @@ -4477,8 +4494,9 @@ class TurnRunner: # fire time (staging repro 2026-07-31: gating registration on # the cache read meant it never registered and no # thread_rename op was ever sent). - agent._on_session_title = lambda title: ( - self._runner._schedule_discord_semantic_thread_rename( + agent._on_session_title = lambda title, title_source: ( + title_source == "llm" + and self._runner._schedule_discord_semantic_thread_rename( source, session_id, title, ) ) @@ -20381,14 +20399,34 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew if not callable(info_fn): return None try: - info = info_fn(str(source.chat_id)) - if ( - isinstance(info, tuple) - and len(info) == 2 - and all(isinstance(x, str) for x in info) - ): - return cast(Tuple[str, str], info) + return _as_thread_info(info_fn(str(source.chat_id))) + except Exception: return None + + async def _await_relay_auto_thread_info( + self, source: SessionSource + ) -> Optional[Tuple[str, str]]: + """``_relay_auto_thread_info``, waited out until this turn delivers. + + The legacy send-result path can only answer once the reply is sent, and + the caller asks at title time — one turn early. The adapter answers on + the send either way, so the timeout is only a backstop for a turn that + never sends at all; the turn's own inactivity limit is exactly how long + that turn could still be alive. + """ + # The connector-stamped prospective id is known at ingest, so most + # sessions answer here and never wait at all. + known = self._relay_auto_thread_info(source) + if known is not None: + return known + adapter = self._adapter_for_source(source) + wait_fn = getattr(adapter, "wait_for_auto_thread_info", None) + if not callable(wait_fn) or not source.chat_id: + return None + # 0 means the operator disabled the turn limit; the backstop still needs one. + timeout = _float_env("HERMES_AGENT_TIMEOUT", 1800) or 1800 + try: + return _as_thread_info(await wait_fn(str(source.chat_id), timeout)) except Exception: return None @@ -20424,18 +20462,15 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew if relay_info is None and not await asyncio.to_thread( self._is_discord_auto_thread_lane, source ): - # Relay title turn with no feedback captured at schedule time: - # the auto-title thread races the delivery that produces the - # connector's send-result feedback (thread_id + initial name). - # Poll the adapter cache briefly before giving up — delivery is - # typically milliseconds-to-seconds behind the title. + # Relay title turn with no feedback captured at schedule time: the + # title comes off the user's opening message, so it beats the + # delivery that produces the connector's send-result feedback + # (thread_id + initial name) by the whole length of the turn. Wait + # on the adapter for that send rather than guessing how long the + # turn will take. if not self._is_relay_discord_channel_lane(source): return - for _ in range(20): # up to ~10s - relay_info = self._relay_auto_thread_info(source) - if relay_info is not None: - break - await asyncio.sleep(0.5) + relay_info = await self._await_relay_auto_thread_info(source) if relay_info is None: # True miss: the connector did not auto-thread this reply # (policy off, DM, already-threaded, or send failed). diff --git a/hermes_cli/models.py b/hermes_cli/models.py index f9714bc307d6e..3a809c5f1a644 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -1695,6 +1695,43 @@ def ai_gateway_model_ids(*, force_refresh: bool = False) -> list[str]: # Cache: maps model_id → {"prompt": str, "completion": str} per endpoint _pricing_cache: dict[str, dict[str, dict[str, str]]] = {} +# A failed fetch caches its empty result too, so an unreachable endpoint isn't +# re-dialed on every call — but only until this deadline. Cached forever, one +# bad moment (a blip during startup, a key that hadn't been written yet) turns +# into no live model discovery for the life of the process, and the processes +# that read this most are the ones that run for weeks: the gateway, the desktop +# backend. Every caller falls back to a curated list meanwhile, so the cost of +# the stale entry is silent and invisible. +_FAILED_CATALOG_TTL_SECONDS = 120.0 +_pricing_cache_retry_after: dict[str, float] = {} + + +def _cached_catalog(cache_key: str) -> Optional[dict[str, dict[str, Any]]]: + """The cached catalog for *cache_key*, or None to go fetch it.""" + cached = _pricing_cache.get(cache_key) + if cached is None: + return None + retry_after = _pricing_cache_retry_after.get(cache_key) + if retry_after is not None and time.monotonic() >= retry_after: + _pricing_cache.pop(cache_key, None) + _pricing_cache_retry_after.pop(cache_key, None) + return None + return cached + + +def _cache_catalog( + cache_key: str, result: dict[str, dict[str, Any]] +) -> dict[str, dict[str, Any]]: + """Cache a catalog result, giving an empty one an expiry.""" + _pricing_cache[cache_key] = result + if result: + _pricing_cache_retry_after.pop(cache_key, None) + else: + _pricing_cache_retry_after[cache_key] = ( + time.monotonic() + _FAILED_CATALOG_TTL_SECONDS + ) + return result + def _format_price_per_mtok(per_token_str: str) -> str: """Convert a per-token price string to a human-friendly $/Mtok string. @@ -1831,8 +1868,10 @@ def fetch_models_with_pricing( ``original``. """ cache_key = (base_url or "").rstrip("/") - if not force_refresh and cache_key in _pricing_cache: - return _pricing_cache[cache_key] + if not force_refresh: + cached = _cached_catalog(cache_key) + if cached is not None: + return cached url = cache_key + "/v1/models" headers: dict[str, str] = { @@ -1847,8 +1886,7 @@ def fetch_models_with_pricing( with _urlopen_model_catalog_request(req, timeout=timeout) as resp: payload = json.loads(resp.read().decode()) except Exception: - _pricing_cache[cache_key] = {} - return {} + return _cache_catalog(cache_key, {}) result: dict[str, dict[str, Any]] = {} for item in payload.get("data", []): @@ -1881,8 +1919,7 @@ def fetch_models_with_pricing( entry["original"] = orig_entry result[mid] = entry - _pricing_cache[cache_key] = result - return result + return _cache_catalog(cache_key, result) def fetch_ai_gateway_pricing( @@ -1899,8 +1936,10 @@ def fetch_ai_gateway_pricing( from hermes_constants import AI_GATEWAY_BASE_URL cache_key = AI_GATEWAY_BASE_URL.rstrip("/") - if not force_refresh and cache_key in _pricing_cache: - return _pricing_cache[cache_key] + if not force_refresh: + cached = _cached_catalog(cache_key) + if cached is not None: + return cached try: req = urllib.request.Request( @@ -1910,8 +1949,7 @@ def fetch_ai_gateway_pricing( with urllib.request.urlopen(req, timeout=timeout) as resp: payload = json.loads(resp.read().decode()) except Exception: - _pricing_cache[cache_key] = {} - return {} + return _cache_catalog(cache_key, {}) result: dict[str, dict[str, str]] = {} for item in payload.get("data", []): @@ -1931,8 +1969,7 @@ def fetch_ai_gateway_pricing( entry["input_cache_write"] = str(pricing["input_cache_write"]) result[mid] = entry - _pricing_cache[cache_key] = result - return result + return _cache_catalog(cache_key, result) def _resolve_openrouter_api_key() -> str: @@ -2039,8 +2076,10 @@ def _fireworks_pricing_from_models_dev( pricing formatter expects per-token strings, so divide by 1M. """ cache_key = "models.dev/fireworks" - if not force_refresh and cache_key in _pricing_cache: - return _pricing_cache[cache_key] + if not force_refresh: + cached = _cached_catalog(cache_key) + if cached is not None: + return cached result: dict[str, dict[str, str]] = {} try: @@ -2068,8 +2107,7 @@ def _fireworks_pricing_from_models_dev( except Exception: result = {} - _pricing_cache[cache_key] = result - return result + return _cache_catalog(cache_key, result) def _fetch_novita_pricing( @@ -2093,8 +2131,10 @@ def _fetch_novita_pricing( base_url = os.getenv("NOVITA_BASE_URL", "").strip() or "https://api.novita.ai/openai/v1" cache_key = base_url.rstrip("/") - if not force_refresh and cache_key in _pricing_cache: - return _pricing_cache[cache_key] + if not force_refresh: + cached = _cached_catalog(cache_key) + if cached is not None: + return cached url = cache_key + "/models" headers = { @@ -2108,8 +2148,7 @@ def _fetch_novita_pricing( with _urlopen_model_catalog_request(req, timeout=timeout) as resp: payload = json.loads(resp.read().decode()) except Exception: - _pricing_cache[cache_key] = {} - return {} + return _cache_catalog(cache_key, {}) result: dict[str, dict[str, str]] = {} for item in payload.get("data", []): @@ -2127,8 +2166,7 @@ def _fetch_novita_pricing( "completion": str(float(out or 0) / 10_000 / 1_000_000), } - _pricing_cache[cache_key] = result - return result + return _cache_catalog(cache_key, result) # All provider IDs and aliases that are valid for the provider:model syntax. diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index ce10a46c9b2e4..803b69c98c027 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -4519,6 +4519,56 @@ class TestFastModelTier: with patch("hermes_cli.models.fetch_models_with_pricing", return_value=catalog): assert ac._fast_model_from_catalog("nous") == "google/gemini-3.6-flash" + def test_catalog_match_skips_the_non_chat_siblings_of_a_chat_model(self): + """A provider names its speech and image endpoints after the chat model + they're paired with, so they satisfy the family rungs and can't answer.""" + from agent import auxiliary_client as ac + + catalog = { + "openai/gpt-4o-mini-tts": {}, + "openai/gpt-4o-mini-transcribe": {}, + "openai/gpt-4o-mini-search-preview": {}, + "openai/gpt-4o-mini": {}, + } + with patch("hermes_cli.models.fetch_models_with_pricing", return_value=catalog): + assert ac._fast_model_from_catalog("nous") == "openai/gpt-4o-mini" + + def test_catalog_match_takes_the_newest_of_a_family(self): + """The bare family rungs must land on the current generation. + + A provider serves every generation of its small tier it hasn't retired, + and compared as strings the oldest sorts first — so the rung meant to + keep the titler current was pinning it to the most obsolete member. + """ + from agent import auxiliary_client as ac + + catalog = { + "openai/gpt-3.5-mini": {}, + "openai/gpt-9-mini": {}, + "openai/gpt-10-mini": {}, + } + with patch("hermes_cli.models.fetch_models_with_pricing", return_value=catalog): + assert ac._fast_model_from_catalog("nous") == "openai/gpt-10-mini" + + def test_catalog_fetch_is_authenticated(self): + """Most /v1/models endpoints need a key; anonymously they 401. + + A 401 reads as "this provider serves no small model", so the titler + would fall back to the curated default and never notice. + """ + from agent import auxiliary_client as ac + + with patch( + "hermes_cli.auth.resolve_api_key_provider_credentials", + return_value={"api_key": "sk-test", "base_url": "https://api.example.com/v1"}, + ), patch( + "hermes_cli.models.fetch_models_with_pricing", return_value={} + ) as fetch: + ac._fast_model_from_catalog("openai") + + assert fetch.call_args.kwargs["api_key"] == "sk-test" + assert fetch.call_args.kwargs["base_url"] == "https://api.example.com" + def test_falls_back_to_curated_default_when_catalog_unavailable(self): """An offline catalog degrades to the provider's pinned default.""" from agent import auxiliary_client as ac diff --git a/tests/agent/test_title_generator.py b/tests/agent/test_title_generator.py index a76342d589b32..56190980b7b71 100644 --- a/tests/agent/test_title_generator.py +++ b/tests/agent/test_title_generator.py @@ -143,7 +143,7 @@ class TestAutoTitleSession: db, "sess-1", "hi", - title_callback=seen.append, + title_callback=lambda title, source: seen.append(title), ) assert db.get_session_title("sess-1") == "Manual Title" @@ -159,12 +159,14 @@ class TestAutoTitleSession: db, "sess-1", "hello", - title_callback=seen.append, + title_callback=lambda title, source: seen.append((title, source)), ) db.set_auto_title.assert_called_once_with( "sess-1", "Readable Session", source="llm" ) - assert seen == ["Readable Session"] + # The stage reaches the consumer, so one that spends a rate-limited + # remote call per title can take this and skip the derived one. + assert seen == [("Readable Session", "llm")] def test_upgrades_a_derived_title_but_not_an_llm_one(self, tmp_path): """The instant title is provisional; a model title is final. @@ -276,6 +278,97 @@ class TestMaybeAutoTitle: assert db.get_session_title("sess-1") is None mock_auto.assert_not_called() + @pytest.mark.parametrize( + "opener", + [ + "[CONTEXT COMPACTION — REFERENCE ONLY] Earlier turns were compacted", + "[CONTEXT SUMMARY]: the user was refactoring the auth module", + "[System note: the user switched models]", + "[Runtime note: resumed from checkpoint]", + ], + ) + def test_skips_every_shape_of_machine_authored_opener(self, tmp_path, opener): + """A session named after our own scaffolding is named after us.""" + db = SessionDB(tmp_path / "state.db") + db.create_session(session_id="sess-1", source="cli") + with patch("agent.title_generator.auto_title_session") as mock_auto: + maybe_auto_title(db, "sess-1", opener, []) + assert db.get_session_title("sess-1") is None + mock_auto.assert_not_called() + + def test_a_multimodal_turn_counts_as_a_real_question(self, tmp_path): + """"Here's a screenshot, fix the login" is a question, parts list or not. + + Judging a turn by `content` alone reads a multimodal one as machinery + and undercounts the conversation, so a session deep into its history + looks like it is still on its opening turn. + """ + from agent.title_generator import _is_real_user_turn + + assert _is_real_user_turn( + { + "role": "user", + "content": [ + {"type": "image_url", "image_url": {"url": "data:image/png;base64,x"}}, + {"type": "text", "text": "fix the login button"}, + ], + } + ) + # An image with no words is not a question we can name anything after. + assert not _is_real_user_turn( + {"role": "user", "content": [{"type": "image_url", "image_url": {"url": "x"}}]} + ) + + def test_titles_on_a_later_turn_when_the_opener_was_not_titleable(self, tmp_path): + """A session whose opener couldn't be titled gets named by a later turn. + + The opener here is a compaction handoff, so turn one leaves the session + nameless. Nothing used to reconsider it: the guard that stops re-titling + a named session also stopped the nameless one from ever asking again. + """ + db = SessionDB(tmp_path / "state.db") + db.create_session(session_id="sess-1", source="cli") + history = [ + {"role": "user", "content": "[CONTEXT COMPACTION — REFERENCE ONLY] x"}, + {"role": "assistant", "content": "ok"}, + {"role": "user", "content": "thanks"}, + {"role": "assistant", "content": "sure"}, + ] + with patch("agent.title_generator.auto_title_session"): + maybe_auto_title(db, "sess-1", "fix the flaky auth test", history) + assert db.get_session_title("sess-1") == "fix the flaky auth test" + + def test_leaves_an_already_titled_session_alone_on_later_turns(self, tmp_path): + """The retry is for nameless sessions only; a named one asks nothing.""" + db = SessionDB(tmp_path / "state.db") + db.create_session(session_id="sess-1", source="cli") + db.set_session_title("sess-1", "Existing name") + history = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi"}, + {"role": "user", "content": "thanks"}, + {"role": "assistant", "content": "sure"}, + ] + with patch("agent.title_generator.auto_title_session") as mock_auto: + maybe_auto_title(db, "sess-1", "and now something else", history) + assert db.get_session_title("sess-1") == "Existing name" + 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'. + + Common openers collide constantly, and the lineage scan that resolves + the collision runs inline on the turn. The model's title lands moments + later, so the session is named either way. + """ + db = SessionDB(tmp_path / "state.db") + db.create_session(session_id="taken", source="cli") + db.set_session_title("taken", "hi") + db.create_session(session_id="sess-1", source="cli") + with patch("agent.title_generator.auto_title_session"): + maybe_auto_title(db, "sess-1", "hi", []) + assert db.get_session_title("sess-1") is None + @@ -284,6 +377,23 @@ class TestMaybeAutoTitle: class TestAutoTitleDuplicateHandling: """Duplicate auto-title handling and not-found hardening (#50537).""" + def test_background_stage_names_a_collision_the_instant_stage_declined( + self, tmp_path + ): + """The lineage scan the turn skipped happens here instead. + + The inline stage declines a collision to stay off the critical path, and + the model can still come back empty. Between them the session would be + left nameless, so the background stage spends the scan the turn wouldn't. + """ + db = SessionDB(tmp_path / "state.db") + db.create_session(session_id="taken", source="cli") + db.set_session_title("taken", "hi") + db.create_session(session_id="sess-1", source="cli") + with patch("agent.title_generator.generate_title", return_value=None): + auto_title_session(db, "sess-1", "hi") + assert db.get_session_title("sess-1") == "hi #2" + def test_dedupes_duplicate_title_via_lineage(self): db = MagicMock() db.get_session_title_source.return_value = None @@ -295,7 +405,12 @@ class TestAutoTitleDuplicateHandling: return_value="Debugging Import Error", ): seen = [] - auto_title_session(db, "sess-1", "hi", title_callback=seen.append) + auto_title_session( + db, + "sess-1", + "hi", + title_callback=lambda title, _source: seen.append(title), + ) db.get_next_title_in_lineage.assert_called_once_with("Debugging Import Error") assert db.set_auto_title.call_args_list[-1][0] == ( "sess-1", @@ -360,3 +475,88 @@ class TestRuntimeValidator: assert called.wait(timeout=10), "auto_title thread never ran" kwargs = mock_auto.call_args.kwargs assert kwargs["runtime_validator"] is _v + + +class TestModelSwitchMarkerNotTitleable: + """Regression: a model-switch marker must never become the session title. + + ``_append_model_switch_marker`` (tui_gateway/server.py) persists its notice + with ``role="user"`` because strict OpenAI-compatible providers reject a + system message that is not first (#48338). Titling therefore has to + recognise it as machine-authored, or switching models before asking the + first real question titles the session + "[System: The active model for this chat has…". + """ + + MARKER = ( + "[System: The active model for this chat has changed to " + "deepseek-v4-flash via provider 94mei. From this point forward, use " + "this runtime metadata when answering questions about what " + "model/provider is active.]" + ) + + def test_marker_prefix_matches_gateway_constant(self): + """The guard must stay in sync with the gateway's marker builder.""" + from tui_gateway.server import _MODEL_SWITCH_MARKER_PREFIX + from agent.title_generator import _MACHINE_PREFIXES + + assert _MODEL_SWITCH_MARKER_PREFIX in _MACHINE_PREFIXES + assert self.MARKER.startswith(_MODEL_SWITCH_MARKER_PREFIX) + + def test_marker_is_not_titleable(self): + from agent.title_generator import is_titleable_user_message + + assert is_titleable_user_message(self.MARKER) is False + + def test_derive_title_is_unguarded_by_design(self): + """``derive_title`` is a dumb formatter; the guard lives in the callers. + + Documents the contract deliberately: every caller checks + ``is_titleable_user_message`` first, so ``derive_title`` itself is + allowed to format a marker. If a future caller forgets that check, the + marker leaks into the title — which is exactly the bug this class + guards against. + """ + from agent.title_generator import derive_title + + assert derive_title(self.MARKER) is not None + + def test_unrelated_system_bracket_text_still_titleable(self): + """The guard is narrow: real user text starting "[System:" still titles.""" + from agent.title_generator import is_titleable_user_message + + assert is_titleable_user_message("[System: my own note] how do I ...") is True + + def test_real_question_after_marker_still_titles(self): + """The marker must not consume the session's one titling opportunity. + + The marker is a role="user" row, so counting it made the first real + question look like turn 2 — and titling bailed out entirely, leaving + the session permanently untitled. + """ + db = MagicMock() + db.get_session_title.return_value = None + db.get_session_title_source.return_value = None + history = [ + {"role": "user", "content": self.MARKER}, + {"role": "user", "content": "南京市秦淮区 小时级天气预报"}, + ] + + 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", "南京市秦淮区 小时级天气预报", history) + assert called.wait(timeout=10), "auto_title never ran after marker" + + def test_instant_title_skips_marker_uses_real_message(self): + from agent.title_generator import apply_instant_title + + db = MagicMock() + db.get_session_title_source.return_value = None + + assert apply_instant_title(db, "sess-1", self.MARKER) is None + assert apply_instant_title(db, "sess-1", "南京市秦淮区 小时级天气预报") == ( + "南京市秦淮区 小时级天气预报" + ) diff --git a/tests/agent/test_turn_context.py b/tests/agent/test_turn_context.py index cf5ff86abf6ef..71f794225519e 100644 --- a/tests/agent/test_turn_context.py +++ b/tests/agent/test_turn_context.py @@ -365,13 +365,44 @@ def test_between_turns_refresh_adds_late_tool_when_servers_registered(): assert any(t["function"]["name"] == "mcp_x_tool" for t in agent.tools) +class _TitlingAgent: + """Only what ``_maybe_title_session_at_turn_start`` reads off an agent.""" + + def __init__(self, platform): + self.platform = platform + self.session_id = "sess-1" + self.model = "test/model" + self.provider = "openrouter" + self.base_url = "https://openrouter.ai/api/v1" + self.api_key = "sk-x" + self.api_mode = "chat_completions" + self._session_db = MagicMock() + self._session_db_created = True +def _title_turn(platform, message="Fix the login button"): + """Run the prologue's titling step and return the maybe_auto_title mock.""" + from agent import turn_context + + with patch("agent.title_generator.maybe_auto_title") as titler: + turn_context._maybe_title_session_at_turn_start( + _TitlingAgent(platform), + [{"role": "user", "content": message}], + ) + return titler +@pytest.mark.parametrize("platform", ["cli", "telegram", "desktop", "acp", None]) +def test_prologue_titles_the_surfaces_a_person_reads(platform): + assert _title_turn(platform).called +@pytest.mark.parametrize("platform", ["cron", "CRON", "subagent"]) +def test_prologue_does_not_title_machine_driven_runs(platform): + """Cron names its own session after the job, and nobody opens a subagent's. - - + Both would otherwise pay a side-LLM call per run for a name that is either + overwritten or never read. + """ + assert not _title_turn(platform).called diff --git a/tests/gateway/relay/test_relay_threads.py b/tests/gateway/relay/test_relay_threads.py index bb1dd45d14c2c..a456f6059efcc 100644 --- a/tests/gateway/relay/test_relay_threads.py +++ b/tests/gateway/relay/test_relay_threads.py @@ -260,6 +260,65 @@ async def test_send_without_thread_feedback_leaves_no_info(): assert adapter.auto_thread_info_for_chat("chan2") is None +@pytest.mark.asyncio +async def test_waiting_for_auto_thread_feedback_outlasts_the_turn(): + """The rename lane asks where the reply landed before the reply exists. + + Titling reads the user's opening message, so the question arrives one whole + turn early — and a turn is however long the agent takes. Waiting on the send + rather than on a fixed nap is what makes the answer arrive at all. + """ + import asyncio + + adapter, stub = _adapter() + + async def send_outbound(action, *, platform=None): + return { + "success": True, + "message_id": "m1", + "thread_id": "th-auto-1", + "auto_thread_name": "What is a duck", + } + + stub.send_outbound = send_outbound # type: ignore[method-assign] + waiting = asyncio.ensure_future(adapter.wait_for_auto_thread_info("chan1", 10.0)) + await asyncio.sleep(0) + assert not waiting.done() + + await adapter.send("chan1", "quack") + assert await waiting == ("th-auto-1", "What is a duck") + + +@pytest.mark.asyncio +async def test_a_reply_that_was_not_auto_threaded_reports_its_miss_on_send(): + """A miss is an answer, and the send is when we have it. + + Only a turn that never sends at all should reach the timeout, so a policy + that doesn't auto-thread costs a wait as long as the turn, not as long as + the backstop. + """ + import asyncio + + adapter, stub = _adapter() + + async def send_outbound(action, *, platform=None): + return {"success": True, "message_id": "m1"} + + stub.send_outbound = send_outbound # type: ignore[method-assign] + waiting = asyncio.ensure_future(adapter.wait_for_auto_thread_info("chan1", 600.0)) + await asyncio.sleep(0) + + await adapter.send("chan1", "quack") + assert await waiting is None + + +@pytest.mark.asyncio +async def test_waiting_for_auto_thread_feedback_gives_up(): + """A turn that never sends anything still has to end.""" + adapter, _stub = _adapter() + assert await adapter.wait_for_auto_thread_info("chan-quiet", 0.05) is None + + @pytest.mark.asyncio async def test_auto_thread_feedback_is_bounded(): adapter, stub = _adapter() @@ -291,6 +350,7 @@ def _mk_runner_stub(): class _Stub: _is_relay_discord_channel_lane = GatewayRunner._is_relay_discord_channel_lane _relay_auto_thread_info = GatewayRunner._relay_auto_thread_info + _await_relay_auto_thread_info = GatewayRunner._await_relay_auto_thread_info _is_discord_auto_thread_lane = GatewayRunner._is_discord_auto_thread_lane _sanitize_discord_thread_title = GatewayRunner._sanitize_discord_thread_title _rename_discord_auto_thread_for_session_title = ( @@ -411,9 +471,12 @@ async def test_sibling_threads_in_one_channel_each_rename_to_own_thread(): @pytest.mark.asyncio -async def test_title_rename_polls_feedback_that_arrives_late(): - """The auto-title races delivery: feedback lands AFTER the rename lane - starts. The lane must poll the adapter cache and still rename.""" +async def test_title_rename_waits_for_feedback_that_arrives_late(): + """The title beats delivery by a whole turn, and the lane still renames. + + Feedback only exists once the reply is sent, so the lane waits on the send + rather than on a nap long enough to cover a turn it can't measure. + """ import asyncio adapter, stub_conn = _adapter() @@ -434,11 +497,21 @@ async def test_title_rename_polls_feedback_that_arrives_late(): runner = _mk_runner_stub()(adapter) src = _relay_channel_source() - async def land_feedback_late(): - await asyncio.sleep(0.7) # past the first poll tick - adapter._auto_thread_by_chat["chan-parent"] = ("th-9", "Initial words") + async def send_outbound(action, *, platform=None): + return { + "success": True, + "message_id": "m1", + "thread_id": "th-9", + "auto_thread_name": "Initial words", + } - task = asyncio.create_task(land_feedback_late()) + stub_conn.send_outbound = send_outbound # type: ignore[method-assign] + + async def deliver_late(): + await asyncio.sleep(0.05) + await adapter.send("chan-parent", "the reply") + + task = asyncio.create_task(deliver_late()) await runner._rename_discord_auto_thread_for_session_title( src, "sess1", "Debugging the flux capacitor" ) @@ -455,11 +528,11 @@ async def test_title_rename_polls_feedback_that_arrives_late(): @pytest.mark.asyncio -async def test_title_rename_true_miss_noops(monkeypatch): - """No feedback ever arrives (connector didn't auto-thread): no rename.""" - import gateway.run as run_mod +async def test_title_rename_true_miss_noops(): + """The connector didn't auto-thread this reply, so there is nothing to rename.""" + import asyncio - adapter, _ = _adapter() + adapter, stub_conn = _adapter() renames: list = [] async def rename_thread(thread_id, name, **kw): @@ -469,14 +542,19 @@ async def test_title_rename_true_miss_noops(monkeypatch): adapter.rename_thread = rename_thread # type: ignore[method-assign] runner = _mk_runner_stub()(adapter) src = _relay_channel_source() - # Shrink the poll loop for test speed: 20 ticks of 0.5s -> patch sleep. - orig_sleep = run_mod.asyncio.sleep - async def fast_sleep(_s): - await orig_sleep(0) + async def send_outbound(action, *, platform=None): + return {"success": True, "message_id": "m1"} - monkeypatch.setattr(run_mod.asyncio, "sleep", fast_sleep) + stub_conn.send_outbound = send_outbound # type: ignore[method-assign] + + async def deliver(): + await asyncio.sleep(0.05) + await adapter.send("chan-parent", "the reply") + + task = asyncio.create_task(deliver()) await runner._rename_discord_auto_thread_for_session_title( src, "sess1", "A title" ) + await task assert renames == [] diff --git a/tests/gateway/test_session_title_rename_lane.py b/tests/gateway/test_session_title_rename_lane.py new file mode 100644 index 0000000000000..691f31c19f9ab --- /dev/null +++ b/tests/gateway/test_session_title_rename_lane.py @@ -0,0 +1,55 @@ +"""Which title stage is allowed to spend a platform rename. + +Titling is two-stage: a derived slice of the user's own words lands inline, and +the model's version replaces it a moment later. A local sidebar wants both. A +Discord thread or a Telegram topic wants only the second — renaming twice lands +on the same name at twice the cost, and Discord allows two channel renames per +ten minutes, so the throwaway can be the one that survives. +""" + +from __future__ import annotations + +import types + +import pytest + +from gateway.config import Platform +from gateway.run import TurnRunner + + +def _attach(lane): + """Attach the title callback for *lane* and return (callback, renames).""" + renames: list = [] + source = types.SimpleNamespace(platform=Platform.DISCORD, chat_id="chan-1") + + runner = types.SimpleNamespace( + _is_telegram_topic_lane=lambda src: lane == "telegram", + _is_discord_auto_thread_lane=lambda src: lane == "discord", + _is_relay_discord_channel_lane=lambda src: False, + _schedule_telegram_topic_title_rename=( + lambda src, sid, title: renames.append(title) + ), + _schedule_discord_semantic_thread_rename=( + lambda src, sid, title: renames.append(title) + ), + ) + holder = types.SimpleNamespace( + _runner=runner, + _attach_session_title_callback=TurnRunner._attach_session_title_callback, + ) + agent = types.SimpleNamespace(session_id="sess-1") + holder._attach_session_title_callback( + holder, agent, types.SimpleNamespace(source=source) + ) + return agent._on_session_title, renames + + +@pytest.mark.parametrize("lane", ["telegram", "discord"]) +def test_the_rename_waits_for_the_model_title(lane): + callback, renames = _attach(lane) + + callback("fix the flaky auth test in log", "derived") + assert renames == [] + + callback("Fix flaky auth test", "llm") + assert renames == ["Fix flaky auth test"] diff --git a/tests/hermes_cli/test_sale_pricing.py b/tests/hermes_cli/test_sale_pricing.py index 359dd75c1295e..fa54797b85146 100644 --- a/tests/hermes_cli/test_sale_pricing.py +++ b/tests/hermes_cli/test_sale_pricing.py @@ -91,3 +91,66 @@ def test_resolve_nous_pricing_credentials_honors_inference_env_override(monkeypa assert base_url == "https://stg-inference-api.nousresearch.com/v1" +def test_a_failed_catalog_fetch_is_not_cached_forever(monkeypatch): + """A blip must not disable live model discovery for the whole process. + + The empty result is cached so a dead endpoint isn't re-dialed on every + call, but it expires — the processes that read this run for weeks, and + every caller silently falls back to a curated list meanwhile. + """ + models_mod._pricing_cache.clear() + models_mod._pricing_cache_retry_after.clear() + + calls = [] + + def _fail(req, timeout=8.0): + calls.append(req) + raise OSError("connection refused") + + monkeypatch.setattr(models_mod, "_urlopen_model_catalog_request", _fail) + + assert fetch_models_with_pricing(base_url="https://example.test") == {} + # Inside the window the failure is cached: no second dial. + assert fetch_models_with_pricing(base_url="https://example.test") == {} + assert len(calls) == 1 + + now = models_mod.time.monotonic() + monkeypatch.setattr( + models_mod.time, + "monotonic", + lambda: now + models_mod._FAILED_CATALOG_TTL_SECONDS + 1, + ) + assert fetch_models_with_pricing(base_url="https://example.test") == {} + assert len(calls) == 2 + + +def test_a_successful_catalog_fetch_stays_cached(monkeypatch): + """Only the failures expire; a real catalog is still fetched once.""" + models_mod._pricing_cache.clear() + models_mod._pricing_cache_retry_after.clear() + + calls = [] + body = json.dumps( + {"data": [{"id": "a/b", "pricing": {"prompt": "1", "completion": "2"}}]} + ).encode() + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = lambda self: self + resp.__exit__ = lambda *a: False + + def _ok(req, timeout=8.0): + calls.append(req) + return resp + + monkeypatch.setattr(models_mod, "_urlopen_model_catalog_request", _ok) + + assert "a/b" in fetch_models_with_pricing(base_url="https://example.test") + now = models_mod.time.monotonic() + monkeypatch.setattr( + models_mod.time, + "monotonic", + lambda: now + models_mod._FAILED_CATALOG_TTL_SECONDS + 1, + ) + assert "a/b" in fetch_models_with_pricing(base_url="https://example.test") + assert len(calls) == 1 + diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 2df2b124e37a4..b5a9f0eacabfe 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -12238,7 +12238,15 @@ def test_prompt_submit_wires_live_title_rename_callback(monkeypatch): hook = getattr(agent, "_on_session_title", None) assert callable(hook), "gateway did not install a live title-rename hook" - hook("Founding of Rome") + # Titling is two-stage, and a local surface wants both: the sidebar renames + # off the derived slice instantly and sharpens when the model's lands. Only + # the lanes that spend a rate-limited remote rename filter by stage. + hook("tell me about rome", "derived") + hook("Founding of Rome", "llm") + assert [payload["title"] for kind, payload in emitted if kind == "session.title"] == [ + "tell me about rome", + "Founding of Rome", + ] assert ( "session.title", {"session_id": "session-key", "title": "Founding of Rome"}, diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 1631441bfbfa8..b9dcb00bff8cf 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -9859,7 +9859,7 @@ def _run_prompt_submit( # sidebar repaints the moment a title lands, rather than waiting # for the next list refresh. _title_key = session.get("session_key") or sid - agent._on_session_title = lambda t, _k=_title_key: _emit( + agent._on_session_title = lambda t, _src, _k=_title_key: _emit( "session.title", sid, {"session_id": _k, "title": t} ) result = agent.run_conversation(run_message, **run_kwargs)