fix(config): accept request_timeout_seconds / stale_timeout_seconds in providers entries (#16779)#16786
Closed
briandevans wants to merge 1 commit into
Closed
Conversation
…n providers entries (NousResearch#16779) The providers-entry validator's `_KNOWN_KEYS` set in `_normalize_custom_provider_entry()` was missing two keys that are both **valid runtime config** and **documented in `cli-config.yaml.example`**: - `request_timeout_seconds` — read by `hermes_cli/timeouts.py:get_provider_request_timeout()` (line 40). - `stale_timeout_seconds` — read by `hermes_cli/timeouts.py:get_provider_stale_timeout()` (lines 65, 69), for both per-model and per-provider levels. Net effect: these keys still worked at runtime (the validator only warns, doesn't strip), but every CLI invocation against the documented example printed a noisy WARNING hermes_cli.config: providers.<name>: unknown config keys ignored: request_timeout_seconds, stale_timeout_seconds That warning trains users to delete keys that are actually load-bearing, so this is more than cosmetic. Fix: add both keys to `_KNOWN_KEYS`. They were already correctly read by `timeouts.py` — only the validator was out of sync with the documented schema and runtime readers. Regression test (`test_timeout_keys_not_warned`): confirms a providers entry with both timeout keys produces zero `"unknown config keys"` warnings. Verified the test fails on plain `origin/main` (warning is emitted as expected) and passes after the one-line fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes provider-config validation noise by aligning _normalize_custom_provider_entry()’s _KNOWN_KEYS with the runtime-supported and documented provider timeout keys, preventing misleading “unknown config keys ignored” warnings for valid settings.
Changes:
- Add
request_timeout_secondsandstale_timeout_secondsto the provider entry_KNOWN_KEYSallowlist. - Add a regression test ensuring these timeout keys do not trigger “unknown config keys” warnings.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
hermes_cli/config.py |
Extends _KNOWN_KEYS in _normalize_custom_provider_entry() to recognize the two timeout keys as valid. |
tests/hermes_cli/test_provider_config_validation.py |
Adds regression coverage asserting no “unknown config keys” warning is emitted when those timeout keys are present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
CI audit — all 19
The PR only widens the |
This was referenced Apr 28, 2026
Closed
Contributor
Author
|
Closing — superseded by @teknium1's #16908 (salvaging @vominh1919's #16853), which landed first and is effectively the same fix. Coverage comparison:
Nothing left to land here. Thanks @vominh1919 and @teknium1. |
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.
Summary
_normalize_custom_provider_entry()'s_KNOWN_KEYSset was missingrequest_timeout_secondsandstale_timeout_seconds, both of which are valid runtime keys and are documented incli-config.yaml.example.unknown config keys ignored: request_timeout_seconds, stale_timeout_secondswarning, even thoughhermes_cli/timeouts.pyreads and applies both keys correctly._KNOWN_KEYS. One-line addition, plus a regression test.The bug
hermes_cli/config.py:2187-2191(pre-fix):But
hermes_cli/timeouts.pyreads:request_timeout_secondsat line 40 (get_provider_request_timeout).stale_timeout_secondsat lines 65 and 69 (get_provider_stale_timeout).And
cli-config.yaml.exampledocuments both keys under the top-levelproviders:block (request_timeout_seconds: 300,stale_timeout_seconds: 900, etc.).So users following the documented example see the validator complain about keys it actually accepts. That's not just noise: a "validator" warning telling users a key is "ignored" trains them to delete keys that are in fact load-bearing for their endpoint behavior.
The fix
Add both keys to the existing
_KNOWN_KEYSset in_normalize_custom_provider_entry(). No other path needs changing — the runtime readers intimeouts.pyalready do the right thing.Test plan
test_timeout_keys_not_warnedintests/hermes_cli/test_provider_config_validation.py— asserts a providers entry with both timeout keys produces zero"unknown config keys"warnings.tests/hermes_cli/test_provider_config_validation.py— 17 passed (16 existing + 1 new)._KNOWN_KEYSchange), the test fails withAssertionError: ... got: ['providers.myhost: unknown config keys ignored: request_timeout_seconds, stale_timeout_seconds']. With the one-line fix applied, the test passes. So the test is genuinely guarding the runtime contract.Related
request_timeout_seconds) and fix(config): add stale timeout settings (salvage #12675) #12891 (stale_timeout_seconds); only the validator was out of sync.