Skip to content

fix: parse JSON-string fallback_providers in YAML config - #63569

Open
hummern wants to merge 2 commits into
NousResearch:mainfrom
turbolego:fix/fallback-json-string-parsing
Open

fix: parse JSON-string fallback_providers in YAML config#63569
hummern wants to merge 2 commits into
NousResearch:mainfrom
turbolego:fix/fallback-json-string-parsing

Conversation

@hummern

@hummern hummern commented Jul 13, 2026

Copy link
Copy Markdown

Problem

When hermes config set writes a list-valued config key like fallback_providers, it serializes the list as a JSON string inside YAML:

fallback_providers: '[{"provider":"opencode-zen","model":"deepseek-v4-flash-free"},...]'

The YAML parser returns this as a str, not a list. _iter_fallback_entries() in fallback_config.py only handled dict and list types — strings fell through to else: return [].

The fallback chain was silently always empty, even when users configured multiple fallback providers including a local llama.cpp model as a final safety net. Every rate limit from the primary provider (e.g., NVIDIA NIM) burned through all retries with zero fallback activation ever logged.

Users saw "The model provider is rate-limiting requests..." despite having a fully working llama.cpp local server running at localhost:8080.

Fix

Added a json.loads() parse guard in _iter_fallback_entries() for strings that start with [ or {. The parsed result is recursively fed back through the same function, so both the legacy single-dict fallback_model format and the multi-entry fallback_providers list format work transparently.

Non-JSON strings fall through to existing empty-return behavior unchanged. YAML-native lists/dicts continue to work as before.

Verification

  • Tested with actual config.yaml containing JSON-string fallback_providers → chain correctly returns 3 entries
  • All existing code paths (dict, list, empty) unchanged
  • get_fallback_chain() integration test passes with both string and list config values
  • All 3 fallback providers resolve correctly via resolve_provider_client()

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #51594 (earliest open, Fixes #51560) — both add a json.loads guard at the top of hermes_cli/fallback_config._iter_fallback_entries for a JSON-string fallback_providers and recurse. Same file, same function, same mechanism (verified by diff). #52099 is the other open duplicate in this cluster; #51594 is canonical.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused shared-helper fix. The premise is confirmed on current main: set_config_value() persists this value as a string at hermes_cli/config.py:8267-8292, while _iter_fallback_entries() returns an empty chain for string values at hermes_cli/fallback_config.py:14-20.

Problems

  • The PR diff changes only hermes_cli/fallback_config.py; it adds no regression coverage for the CLI persistence path described in the PR. This is a config-propagation change, for which AGENTS.md:84-87 calls for exercising the real path.

Suggested changes

  • Add an isolated-HERMES_HOME test that calls set_config_value(), reloads the config, and verifies get_fallback_chain() returns the configured JSON-string entries in order.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 16, 2026
When Hermes config set writes a list-valued key like fallback_providers,
it serializes as a JSON string inside YAML. The YAML parser returns a
string, not a list. _iter_fallback_entries() only handled dict and list
types, silently returning empty for strings — which meant the fallback
chain never populated and users hit rate-limit dead ends even with
multiple fallback providers (including a local llama.cpp model) configured.

Add a json.loads() parse guard in _iter_fallback_entries() for strings
that look like JSON arrays/objects. The recursion handles both the
single-dict legacy fallback_model shape and the multi-entry
fallback_providers list shape transparently.

Fixes: sessions stuck on rate-limited providers with no fallback activation
@turbolego
turbolego force-pushed the fix/fallback-json-string-parsing branch from dc14077 to 9b702b9 Compare August 2, 2026 05:16
@turbolego
turbolego force-pushed the fix/fallback-json-string-parsing branch from 9b702b9 to 275cad5 Compare August 2, 2026 05:26
@turbolego

Copy link
Copy Markdown

@teknium1 Added test_fallback_config.py with TestGetFallbackChainIntegration covering the full set_config_value()load_config()get_fallback_chain() path against an isolated HERMES_HOME. Tests exercise the JSON-string round-trip (the regression case), order preservation, the YAML-native list path, and the empty-config baseline. Also rebased onto current main and merged with the existing TestResolveEntryApiKey tests from #43861/#74311.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants