fix(moa): parse JSON string reference_models in _normalize_preset - #59497
Closed
wen0531 wants to merge 1 commit into
Closed
fix(moa): parse JSON string reference_models in _normalize_preset#59497wen0531 wants to merge 1 commit into
wen0531 wants to merge 1 commit into
Conversation
When reference_models is stored as a JSON string (e.g. from hermes moa configure or hand-edited config.yaml), _normalize_preset silently falls back to hardcoded defaults because the string fails both isinstance(x, list) and isinstance(x, dict) checks. Add json.loads() parsing before the type checks so both formats work.
teknium1
reviewed
Jul 15, 2026
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the focused normalization fix. Current main does exhibit the fallback: hermes_cli/moa_config.py:132-141 treats a string reference_models value as invalid and substitutes defaults.
Problems
- The PR has no regression test.
tests/hermes_cli/test_moa_config.py:82-97covers numeric scalars and bare mappings, but not a valid JSON string containing model slots. - The stated
hermes moa configureroot cause does not match current code:hermes_cli/moa_cmd.py:100-117constructsrefsas a native list, assigns it directly, normalizes it, then saves it. The confirmed use case is hand-edited or otherwise pre-existing JSON-string config.
Suggested changes
- Add a normalization test for a JSON-string list of valid slots, plus malformed JSON fallback behavior.
- Reframe the PR description around configuration tolerance rather than CLI serialization.
This is an automated hermes-sweeper review.
|
|
||
| raw_refs = raw.get("reference_models") | ||
| # reference_models may be a JSON string (hand-edited config.yaml) or a list. | ||
| if isinstance(raw_refs, str): |
Contributor
There was a problem hiding this comment.
Please add a regression test in tests/hermes_cli/test_moa_config.py covering a valid JSON-string list of reference slots (and malformed JSON fallback), since the existing non-list tests only cover scalar and mapping inputs.
teknium1
added a commit
that referenced
this pull request
Jul 23, 2026
Contributor
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
hermes moa listshows hardcoded default models (openai-codex:gpt-5.5, openrouter:deepseek/deepseek-v4-pro) instead of user-configured presets.Root Cause
hermes moa configureserializesreference_modelsas a JSON string in config.yaml:But
_normalize_presetexpects a native list — it checksisinstance(raw_refs, list)which fails for strings, then falls back toDEFAULT_MOA_REFERENCE_MODELS.Fix
Add
json.loads()parsing before the type check in_normalize_preset, so both JSON string and native list formats work correctly.Verification
Before fix:
After fix: