Skip to content

fix(streaming): respect auxiliary.title_generation config for session titles - #931

Merged
nesquena-hermes merged 1 commit into
masterfrom
pr-925-review
Apr 24, 2026
Merged

nesquena-hermes merged 1 commit into
masterfrom
pr-925-review

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Fixes title generation ignoring auxiliary.title_generation config. When a user configures a dedicated title model in config.yaml, it was silently overridden by the chat session's model.

Changes

api/streaming.py:

  • _aux_title_configured() — detects whether auxiliary.title_generation has any meaningful config (provider ≠ auto, model, or base_url set)
  • _aux_title_timeout() — reads per-task timeout from config; previously hardcoded to 15.0 s
  • _generate_llm_session_title_via_aux() — adds use_agent_model kwarg to preserve existing behavior when no dedicated config exists
  • _run_background_title_update() — when aux title is configured, routes directly to aux; adds missing llm_invalid_aux fallback path

tests/test_title_aux_routing.py — 23 new tests covering all branches.

Test results

23/23 pass. Full suite: 2088 passing, 0 failed.

Co-authored by @starship-s. Replaces PR #925 (branch had been behind master).

… titles

- _aux_title_configured(): returns True when provider/model/base_url is set
- _aux_title_timeout(): reads configured timeout, falls back to 15.0s default
- _generate_llm_session_title_via_aux: use_agent_model kwarg preserves old behavior
- Missing llm_invalid_aux fallback now triggers agent-model retry
- 23 new tests in tests/test_title_aux_routing.py — all pass

Co-authored-by: starship-s <starship-s@users.noreply.github.com>

@nesquena nesquena left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — end-to-end ✅ (clean, no fixes needed)

Traced against upstream hermes-agent

Fresh nousresearch/hermes-agent tarball pulled. Walked the whole routing chain:

_resolve_task_provider_model at agent/auxiliary_client.py:2439-2489 — priority order is:

  1. Explicit base_url arg → forces "custom" provider with that endpoint
  2. Explicit provider arg → wins over task config
  3. cfg_base_url (from auxiliary.<task>.base_url)
  4. cfg_provider (if not "auto")
  5. Auto-detection fallback

The PR relies on the semantics that passing None for provider/model/base_url lets the resolver read them from config.yaml's auxiliary.title_generation section. Line 2472: resolved_model = model or cfg_model — explicit model wins, otherwise config.

The PR correctly handles this with provider or None / model or None / base_url or None at api/streaming.py:305-307, converting empty strings (the default when use_agent_model=False) to None so the resolver falls through to config-based resolution.

_get_auxiliary_task_config at auxiliary_client.py:2495-2506 — returns {} on ImportError; always a dict. Matches webui _aux_title_configured's safety wrapper.

call_llm at auxiliary_client.py:2693-2735 — accepts explicit timeout: float = None (None → reads from _get_task_timeout(task)). When the webui passes timeout=_timeout (a concrete float), that explicit value wins. ✓

End-to-end trace of the four fixes

#1 _aux_title_configured() (streaming.py:236-246). Returns True when any of provider (non-auto), model, or base_url is meaningfully set. Three or-gated conditions. Exception-safe (returns False on ImportError). Correctly case-insensitive on provider via .lower() != 'auto'. ✓

#2 _aux_title_timeout() (streaming.py:248-270). Reads auxiliary.title_generation.timeout, coerces to float, rejects None / non-numeric / zero / negative — falls back to default (15.0s) with a debug log. Slightly stricter than upstream _get_task_timeout (which accepts zero/negative). ✓

#3 use_agent_model kwarg on _generate_llm_session_title_via_aux (streaming.py:429-456).

  • use_agent_model=False (new default): pass provider='', model='', base_url=''provider or None → None → upstream resolver reads from config
  • use_agent_model=True (legacy fallback): pass agent's attrs → explicit args win, config ignored — matches pre-PR behavior for the no-aux-config case

Correctly preserves the previous "try agent model first" behavior when auxiliary.title_generation isn't configured.

#4 Routing change in _run_background_title_update (streaming.py:578-588).

Case matrix traced:

State Route tried Fallback
No aux config + agent present agent-model title aux with agent's model (legacy)
Aux configured + agent present aux via config agent-model title
No agent (regardless of aux config) aux via config (none — agent unavailable)

Also adds llm_invalid_aux to the fallback trigger set (was only llm_error_aux before). Now both llm_error_aux (network/API error) and llm_invalid_aux (response didn't parse to a valid title) correctly trigger the agent-model fallback. ✓

Cross-tool (CLI) check

auxiliary.title_generation is a shared config.yaml section. The PR reads the SAME keys the hermes-agent itself reads for its own title generation via _get_auxiliary_task_config('title_generation'). Both tools converge on the same interpretation:

  • provider='openai', model='gpt-4o-mini' → both use OpenAI gpt-4o-mini for title generation
  • base_url='http://local-llm:8080/v1' → both route through that endpoint

No cross-tool divergence. The PR actually improves cross-tool consistency — before, webui ignored the auxiliary config while the CLI honored it. Now both honor it. ✓

Security audit

No new user input surface, no new CDN deps, no auth-related changes. The new code reads config.yaml (a trusted file on disk) and passes primitive values (string, float) to call_llm. No injection vectors. ✓

Edge-case trace

Scenario Behaviour
Fresh install, no auxiliary.title_generation in config _aux_title_configured() returns False → agent-model path wins (same as master) ✅
auxiliary.title_generation: {provider: openai, model: gpt-4o-mini} Aux route fires → config.yaml values resolve via upstream _resolve_task_provider_model
auxiliary.title_generation: {provider: auto} _aux_title_configured() returns False (auto = not meaningful) → agent-model path. Intentional — auto means "use default routing" ✅
auxiliary.title_generation: {base_url: http://localhost:1234/v1} Returns True (base_url is set) → aux route uses that endpoint as "custom" per upstream line 2483 ✅
auxiliary.title_generation: {provider: openai} (model blank) Returns True → aux route uses openai with provider-default model ✅
auxiliary.title_generation: {timeout: 30} (int) _aux_title_timeout() coerces to 30.0 ✅
auxiliary.title_generation: {timeout: 0} or -1 or 'abc' or missing Falls back to 15.0 default ✅
Aux route returns llm_error_aux (API error) Agent fallback triggers ✅
Aux route returns llm_invalid_aux (title didn't parse) Agent fallback triggers ✅ (was missing before this PR)
Aux route succeeds Agent route NOT called ✅
agent.auxiliary_client import fails _aux_title_configured() returns False → agent path; call_llm also fails with ImportError at call-site → caught by existing except in generate_title_raw_via_aux
Concurrent writes to config.yaml during title generation Stale read possible but acceptable — title gen is a background task; worst case is using previous config for one turn
use_agent_model=True (legacy fallback) Passes agent's provider/model/base_url as explicit overrides → they win per upstream priority 1 → exact pre-PR behavior preserved ✅

Tests

  • 23/23 pass in new tests/test_title_aux_routing.py
  • Full local suite: 2041 passed, 47 skipped, 0 failed

Test coverage is thorough — _aux_title_configured tested across 10 config shapes (model-only, base_url-only, provider-only, auto case, empty, provider+blank-model, etc.); timeout-in-config verified via a fake call_llm that captures the kwarg; edge cases for zero/negative/non-numeric/empty-string; llm_error_aux and llm_invalid_aux fallback cases both covered via mocked session fixture + event capture.

Other audit — things that are correct already

  • Upstream semantics verified: I confirmed _resolve_task_provider_model at auxiliary_client.py:2439-2489 treats None / empty provider-or-base_url as "fall through to task config" — exactly what the PR relies on. ✓
  • Timeout plumbing: webui resolves timeout locally (stricter semantics than upstream's _get_task_timeout) and passes the concrete value to call_llm. Upstream uses the explicit value unchanged. ✓
  • Exception safety: both helpers (_aux_title_configured, _aux_title_timeout) wrap in try/except and fall back to safe defaults. ✓
  • Case-insensitive auto check: provider.lower() != 'auto' covers 'auto', 'Auto', 'AUTO', etc. ✓
  • Agent lock: the session s.title write at streaming.py:592 is already wrapped in _get_session_agent_lock(session_id) from PR #910. The PR just removed some dead code comments around that section but the locking semantics are preserved. ✓
  • Agent path unchanged: _generate_llm_session_title_for_agent and the status codes llm_error / llm_invalid are unchanged — the PR only routes around them. ✓

Minor observations (non-blocking)

  • Duplicated timeout logic: _aux_title_timeout (stricter: rejects zero/negative) duplicates upstream _get_task_timeout (accepts any numeric). Could be simplified to from agent.auxiliary_client import _get_task_timeout; _get_task_timeout('title_generation', default=15.0) and just pass timeout=None to call_llm to let upstream read it natively. Trade-off: webui's stricter semantics are lost. Non-urgent — the duplication is clearly documented and the tests lock the behavior.

  • Default drift: webui default = 15s, upstream _DEFAULT_AUX_TIMEOUT = 30s. Intentional — titles should be quick. Non-issue.

  • use_agent_model is a keyword-only arg (*, use_agent_model: bool = False) — good practice; can't be passed positionally by accident.

  • _aux_title_configured() is called once per title update, which is fine, but each call re-imports agent.auxiliary_client and re-parses config.yaml. For high-volume title updates the load_config() call might show up in profiles. Not urgent — it's a background path and upstream likely caches the config.

Recommendation

Clean, correctly scoped fix for a real config-ignored bug. End-to-end trace against the upstream _resolve_task_provider_model semantics confirms the provider or None pattern correctly defers to config.yaml when no explicit args are passed. Tests cover all the branches the routing change touches. No cross-tool or security concerns. Nothing to push back on.

Approved. Ready for merge + v0.50.182 tag.

@nesquena-hermes
nesquena-hermes merged commit 57222c7 into master Apr 24, 2026
3 checks passed
nesquena-hermes added a commit that referenced this pull request Apr 24, 2026
…oning chip

Rebased onto master after #931 (aux title routing) to resolve streaming.py conflict.
All changes from both PRs are cleanly integrated.

2088 tests passing (2065 master + 23 from #931).

Co-authored-by: bergeouss <bergeouss@gmail.com>
@nesquena-hermes
nesquena-hermes deleted the pr-925-review branch April 24, 2026 01:35
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
fix(streaming): respect auxiliary.title_generation config for session titles
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
…oning chip

Rebased onto master after nesquena#931 (aux title routing) to resolve streaming.py conflict.
All changes from both PRs are cleanly integrated.

2088 tests passing (2065 master + 23 from nesquena#931).

Co-authored-by: bergeouss <bergeouss@gmail.com>
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
fix(streaming): respect auxiliary.title_generation config for session titles
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…oning chip

Rebased onto master after nesquena#931 (aux title routing) to resolve streaming.py conflict.
All changes from both PRs are cleanly integrated.

2088 tests passing (2065 master + 23 from nesquena#931).

Co-authored-by: bergeouss <bergeouss@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants