refactor(inventory): extract shared ConfigContext + build_models_payload

[kshitijk4poor]

What this PR does

Three call-sites in the codebase each duplicated the same config-slice + list_authenticated_providers + post-processing pattern:

• hermes_cli/web_server.py /api/model/options (dashboard)
• tui_gateway/server.py model.options JSON-RPC
• tui_gateway/server.py model.save_key JSON-RPC

This consolidates them onto a new hermes_cli/inventory.py:

load_picker_context() -> ConfigContext
    # Replaces the 17-LOC config-slice (model.{default,name,provider,
    # base_url}, providers:, custom_providers:) every consumer did inline.

ConfigContext.with_overrides(*, current_provider=, current_model=,
                             current_base_url=) -> ConfigContext
    # Truthy-only overlay for TUI agent-session state on top of disk
    # config. Empty getattr(agent, ...) attrs MUST NOT clobber disk.

build_models_payload(ctx, *, include_unconfigured=False,
                     picker_hints=False, canonical_order=False,
                     max_models=50) -> dict
    # Single payload builder. Delegates curation to
    # list_authenticated_providers (does not call provider_model_ids
    # per row — that pulls non-agentic models like TTS/embeddings).
    # picker_hints + canonical_order produce the TUI shape;
    # defaults match the dashboard's existing /api/model/options
    # contract.

Site	Before	After
/api/model/options	30-line inline config dance + raw cfg.get("custom_providers") (which misses v12+ keyed providers — see Bug A below) + list_authenticated_providers	build_models_payload(load_picker_context(), max_models=50)
TUI model.options	17-line config dance + list_authenticated_providers + 45-line canonical-merge / picker-hint post-processing	build_models_payload(ctx, include_unconfigured=True, picker_hints=True, canonical_order=True, max_models=50) with with_overrides() for agent-session state
TUI model.save_key	Inline list_authenticated_providers + slug match	build_models_payload(ctx, picker_hints=True) then slug match

Scope: 3 of 5 inventory call-sites

Issue #23359 named four call-sites (plus one this PR found in gateway/run.py). This PR migrates 3 of 5:

Call-site	Status
hermes_cli/web_server.py /api/model/options	✅ migrated
tui_gateway/server.py model.options	✅ migrated
tui_gateway/server.py model.save_key	✅ migrated
cli.py:6294 interactive picker	⏭ left on legacy path (interactive flow, lower duplication-removal value)
hermes_cli/main.py:2269 aux-task switcher	⏭ left on legacy path
gateway/run.py:8999 gateway model switcher	⏭ left on legacy path

The remaining 3 call-sites would migrate cleanly to build_models_payload() in a follow-up PR; they're stable enough that there's no urgency, and bundling everything would expand this PR's scope past what reviewers can reasonably check in one pass.

One latent bug fixed; one regression guard added

1. Bug A (real, fixed in 3 sites): web_server.py:998, tui_gateway/server.py:5178 (model.options), and tui_gateway/server.py:5312 (model.save_key) all read cfg.get("custom_providers") directly on main, missing the v12+ keyed providers: form. Routing all 3 sites through load_picker_context() (which calls get_compatible_custom_providers()) fixes the dashboard + both TUI handlers in one go.

2. Regression guard against canonical-slug demotion (preventive, not curative): Section 3 of list_authenticated_providers sets is_user_defined=True on rows from the providers: config dict even when the slug is canonical. If a future refactor naively keys row ordering on is_user_defined, canonical providers would silently sink to the picker tail. Current tui_gateway/server.py:5194 already keys on slug membership correctly (landed in commit c23c7c994 "fix(tui): address remaining review feedback — ordering and digit shortcuts"). _reorder_canonical preserves that invariant in the new module, and test_canonical_order_uses_slug_not_is_user_defined_flag locks it as a regression guard. The bug doesn't currently exist; this prevents it from being reintroduced.

Why this PR (vs #23369)

This replaces the rejected #23369, which bundled this consolidation with three new scriptable CLI surfaces (hermes models list, hermes models status, hermes providers list), an offline-mode synthesis path, a versioned JSON contract, and text renderers — none of which have external user demand. That PR was +1638/-140; only ~230 LOC of it served the consolidation goal.

This PR is just the refactor. The scriptable CLI is deferred until a real user asks. Issue #23359 stays open after this lands — the consolidation half is addressed; the scriptable-CLI half is deferred pending external demand.

Stats

• +666 / -145 (net +521)
• Module: 240 LOC (vs 600 in feat: hermes models / hermes providers — non-interactive inventory CLI (#23359) #23369)
• Tests: 18 behavior tests, 378 LOC (vs 814 in feat: hermes models / hermes providers — non-interactive inventory CLI (#23359) #23369)
• Consolidation ratio: 145 / 240 = 0.60 (above the 0.5 threshold for honest consolidation accounting)
• Per-file numstat: inventory.py +240/-0, web_server.py +2/-32, tui_gateway/server.py +46/-113, test_inventory.py +378/-0

What this PR is NOT doing

• Not adding new CLI commands or JSON contracts.
• Not adding offline mode (--offline) — --no-live already covers cron/CI use cases via curated lists.
• Not changing list_authenticated_providers itself — the substrate is unchanged; this only consolidates how three callers invoke it.
• Not touching the interactive picker (cli.py), the aux-task switcher (hermes_cli/main.py), or the gateway model switcher (gateway/run.py).
• Not addressing tracking: provider/model inventory has no scriptable surface — five PRs reinvent it, four issues blocked #23359's "scriptable inventory" framing — that's deferred until external demand surfaces.

Test plan

• 18 new tests in tests/hermes_cli/test_inventory.py lock the public API contract:
  • load_picker_context() reproduces the inline 17-LOC config-slice exactly (full dict, name fallback, legacy bare-string model, empty config).
  • with_overrides() is truthy-only (empty agent attrs preserved).
  • build_models_payload() returns the expected {providers, model, provider} shape and does NOT call provider_model_ids per row.
  • include_unconfigured=True appends CANONICAL_PROVIDERS skeleton rows without duplicating already-present slugs.
  • picker_hints=True adds authenticated/auth_type/key_env/warning and the api_key warning format references the actual env var (e.g. ANTHROPIC_API_KEY).
  • Regression guard: canonical_order keys on slug membership, NOT is_user_defined — canonical slugs configured via the keyed providers: schema must NOT be demoted to the tail.
  • End-to-end credential-leak canary: OPENROUTER_API_KEY / ANTHROPIC_API_KEY values never appear in the JSON-serialized payload, even with picker_hints=True.
  • Frontend shape compatibility: every row exposes the keys web/src/components/ModelPickerDialog.tsx reads (name, slug, models, total_models, is_current, authenticated).
• Live invocation verified:
  • web_server.get_model_options() → 9 authenticated rows, clean shape, current_provider/current_model populated correctly.
  • TUI model.options JSON-RPC → 36 rows (full universe with hints + canonical order), warning hint correctly formatted.
• Regression sweep on tests/hermes_cli/ + tests/test_tui_gateway_server.py: 4379 passed, 17 failed. The 17 failures reproduce identically on origin/main (gateway systemd / WSL detection / custom-provider grouping / plugin discovery / web-server auth / hangup protection — all pre-existing, none introduced by this PR).
• Lint diff: ruff clean. ty clean for hermes_cli/inventory.py.

Known test gaps (acknowledged, not blockers)

• No parity test asserting build_models_payload output equals the OLD inline pipeline output for shared inputs. Behavioral coverage is solid via individual feature tests; if the inline slice had a quirk (e.g. or "" precedence on an unusual model dict), parity tests would catch it. Worth adding in a follow-up if reviewers want belt-and-braces.
• No directed test for model.save_key post-save populated-models contract. The credential-leak canary exercises the real curation path but doesn't assert the freshly-keyed provider has non-empty models. Low risk in practice (env var is set into os.environ before the rebuild call) but uncovered in tests.
• The "no provider_model_ids per row" assertion runs with default flags, not the production TUI shape (include_unconfigured=True, picker_hints=True, canonical_order=True). Risk is bounded because _apply_picker_hints and _reorder_canonical are pure transformations, but a one-line additional assertion would harden it.

Refs

• Replaces feat: hermes models / hermes providers — non-interactive inventory CLI (#23359) #23369 (closed simultaneously).
• Refs tracking: provider/model inventory has no scriptable surface — five PRs reinvent it, four issues blocked #23359 — addresses the consolidation half; tracking: provider/model inventory has no scriptable surface — five PRs reinvent it, four issues blocked #23359 stays open for the scriptable-CLI half.

[alt-glitch]

@daimon-nous review

[daimon-nous]

Review: refactor(inventory) — extract shared ConfigContext + build_models_payload

Verdict: ✅ LGTM — clean, well-scoped refactor with a real bug fix included.

What it does

Extracts the duplicated "config-slice → list_authenticated_providers → post-process" pattern from 3 call-sites into a new hermes_cli/inventory.py module (240 LOC). The 3 consumers collapse to one-liners calling build_models_payload(load_picker_context(), ...).

Bug fix (real, confirmed on main)

All three pre-existing call-sites used cfg.get("custom_providers") which only reads the legacy list format. The new load_picker_context() routes through get_compatible_custom_providers(cfg) which also picks up v12+ keyed providers: entries. This is a genuine behavioral improvement for users who configure providers via the keyed schema.

Code quality observations

1. ConfigContext.with_overrides() is well-designed — truthy-only semantics correctly handle the TUI's empty getattr(agent, "provider", "") when no agent is spawned yet. Immutable dataclass + replace() is clean.

2. _apply_picker_hints heuristic is sound — distinguishes authenticated rows from skeleton rows via source == "canonical" and not models. This is correct because:

   • Section 2 of list_authenticated_providers emits source: "canonical" rows WITH populated models (they passed credential checks)
   • _append_unconfigured_rows emits source: "canonical" rows with EMPTY models (skeleton placeholders)

   Edge case handled: section 1 rows use source: "built-in" → correctly marked authenticated.

3. _reorder_canonical keys on slug membership, not is_user_defined — correct design decision, well-documented in both code comments and a regression test. Prevents the canonical-slug-demotion bug.

4. Test coverage is solid (18 tests, 378 LOC) — covers load_picker_context variants, with_overrides semantics, build_models_payload shape, picker_hints behavior, canonical_order regression, credential-leak canary, and frontend shape compatibility.

Minor observations (not blocking)

• _append_unconfigured_rows uses .lower() for slug comparison in the seen set, while _reorder_canonical uses exact match. Functionally equivalent since all canonical slugs are lowercase, but slightly inconsistent. Not worth changing.

• The load_config() vs _load_cfg() swap: the TUI gateway previously used its own _load_cfg() caching. Now it goes through hermes_cli.config.load_config() (also mtime-cached). Both do mtime-based staleness checks, so no performance regression. This is actually an improvement — one canonical config loader.

• The PR correctly leaves 3 other call-sites unmigrated (cli.py interactive picker, hermes_cli/main.py aux-task switcher, gateway/run.py model switcher) for a follow-up. Good scope discipline.

CI status

3 checks fail (test, e2e, Windows footguns) — all reproduce identically on origin/main. Not introduced by this PR.

Summary

Well-scoped refactor that reduces duplication across 3 inventory call-sites, fixes a real v12+ provider visibility bug, and adds a regression guard for canonical-slug ordering. The consolidation ratio (145 deleted / 240 added = 0.60) is honest. Replaces the rejected #23369 which bundled unnecessary CLI surfaces. Ready to merge.

[teknium1]

Salvaged onto current main via #25450. Your inventory module and tests cherry-picked with authorship preserved; we also extended the consolidation to the CLI /model picker (cli.py) — fourth call-site on the legacy path that fits cleanly. The aux-task and gateway switchers stay on legacy because their config semantics differ. Thanks!