Skip to content

fix(config): parse structured list/dict values in hermes config set (consolidates 8-PR cluster) - #88163

Merged
teknium1 merged 2 commits into
mainfrom
fix/config-set-structured-values
Aug 17, 2026
Merged

fix(config): parse structured list/dict values in hermes config set (consolidates 8-PR cluster)#88163
teknium1 merged 2 commits into
mainfrom
fix/config-set-structured-values

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

hermes config set now parses structured list/dict values (JSON flow literals and multi-line YAML blocks) into real YAML structures instead of writing them verbatim as quoted strings — while keeping string-typed keys (e.g. approvals.mode) exempt from all coercion.

Changes

Validation

Check Result
pytest tests/hermes_cli/test_config_set_list_values.py tests/hermes_cli/test_set_config_value.py -o addopts="" -q (py3.11, isolated HERMES_HOME) 95 passed
Sabotage-verify (new test file against main's config.py) 7 failed without the fix, all pass with it
ruff check on touched files All checks passed
String-typed-key guard approvals.mode '[off]' and '-5' stay strings (tested)
Round-trip structured value survives load_config as a real list (tested)

Cluster

This PR consolidates an 8-PR duplicate cluster, all addressing "config set stores structured values as strings":

Thanks to all eight contributors — @liuhao1024 reported/fixed this earliest; #59182 was picked as the base because it was the only variant already folded inside the string-typed-key guard.

Infographic

PR infographic

sam7894604 and others added 2 commits August 16, 2026 22:10
Fold the list/mapping parser INSIDE the existing string-typed-value coercion guard (the `not isinstance(_default_value_for_key(key), str)` block from e4ea0a0) instead of running it unconditionally, so a genuinely string-typed setting whose value merely starts with '[' or '{' is left untouched while non-string keys get JSON/YAML flow literals parsed to real lists/dicts.

Update website/docs/user-guide/configuring-models.md: the `config set only writes scalar values` note is no longer accurate; document the list/mapping support with a quoted example.

Fixes #40545 #50168
…s with a conservative trigger

Consolidation follow-up on top of #59182's cherry-picked base:

- Add _looks_structured_value(): triggers a yaml.safe_load structured
  parse only when the value starts with '[' / '{' or spans multiple
  lines with YAML list-item ('- x') or mapping-entry ('key: v') shaped
  lines. Deliberately avoids the over-broad leading '-' trigger from
  #88066 so '-5' and '--flag' stay strings.
- Stays folded INSIDE the string-typed-key guard: keys whose
  DEFAULT_CONFIG type is str (e.g. approvals.mode) are never coerced.
- Tests: multi-line YAML list/dict, string-typed key given '[x]' and
  '-5' stays string, dash-prefixed scalars stay strings, plain
  multi-line prose stays a string, load_config round-trip.
  Sabotage-verified: 7 of the suite's tests fail on main without the fix.
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on 0e1fac7 — fix(config): extend structured-value parsing to multi-line Y

⚠️ Warnings

OSV vulnerability scan · View job

5 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.


debug info

CI timings

CI timings · View report · View job

Wall time 3m36s vs 3m35s (+0.5%). 14 job(s) slower, 10 faster, 1 unchanged.

  • Python tests / Run tests slice 3/12: +50.0s
  • Python tests / Run tests slice 5/12: +47.0s
  • Python tests / Run tests slice 8/12: +24.0s
  • Python tests / Run tests slice 7/12: +23.0s
  • Python tests / Run tests slice 4/12: -20.0s

@teknium1
teknium1 merged commit acaac9a into main Aug 17, 2026
45 checks passed
@teknium1
teknium1 deleted the fix/config-set-structured-values branch August 17, 2026 05:18
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists 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 labels Aug 17, 2026
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 P2 Medium — degraded but workaround exists 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.

3 participants