feat: Config-Runtime Contract Registry (Phase 1) + fix #28046 + fix #28863 - #28995
feat: Config-Runtime Contract Registry (Phase 1) + fix #28046 + fix #28863#28995zccyman wants to merge 1 commit into
Conversation
…8046 + fix NousResearch#28863 Adds a declarative config binding registry that maps config.yaml keys to their runtime consumers. On gateway startup, validates all registered bindings are active — catching orphan config fields before users discover them silently. Changes: - New module: hermes_cli/config_contracts.py - ConfigBinding dataclass + CONFIG_BINDINGS registry (29 bindings) - get_nested() resolves dotted/wildcard keys including list-of-dicts - validate_config_bindings() cross-checks config vs registry at startup - get_binding_report() for /info debugging - Fix NousResearch#28046: Read max_tokens from custom_providers per-model config - Extract get_custom_provider_model_field() as generic lookup helper - Add get_custom_provider_max_tokens() symmetric to context_length - Read custom_providers max_tokens in agent_init.py - 10 regression tests - Fix NousResearch#28863: Add docker_extra_args to _terminal_env_map - The key was in DEFAULT_CONFIG but missing from the env var bridge - Gateway startup: inject contract validation (warnings only, no behavior change) - Tests: 17 new (10 max_tokens + 17 contracts = 27 unique, 39 total with existing context_length tests) Implements: NousResearch#28984 (Typed Config-Runtime Contract, Phase 1)
|
Note: The two bug fixes bundled here have existing dedicated PRs:
The Config-Runtime Contract Registry feature implements #28984. |
|
Thanks @alt-glitch for the review. Acknowledged on the bundled bug fixes. My reasoning for keeping them together: #28142 and #28891 are still open (not merged), so the fixes aren't yet available to users. The feature work (config_contracts.py) was motivated by these same bugs, so bundling them here provides a complete solution in one PR. However, I'm happy to split if you prefer. The options are:
Which approach do you prefer? If you want a split, let me know and I'll remove the bug fix commits. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for putting the structural config-contract idea together. I verified the premise against current main, and the feature is still not implemented there, but this PR needs real cleanup before it would be safe to salvage.
Problems
hermes_cli/config_contracts.py:138-180never appends towarnings, sovalidate_config_bindings()always returns[]; the gateway hook atgateway/run.py:664-667would not catch orphan config fields.hermes_cli/config_contracts.py:183-196makes strict mode a no-op too:_check_unregistered_keys()computesfull_keybut never compares or reports it.- The PR sets custom-provider
max_tokensatagent/agent_init.py:1268, after current main snapshots_session_init_model_configatagent/agent_init.py:1343; sessions created viarun_agent.py:515-520would record stalemax_tokensmetadata. .dev-workflow/code-graph.dbis an unrelated binary artifact.- The
docker_extra_argsbridge hunk is stale: current main already hasTERMINAL_CONFIG_ENV_MAPwithdocker_extra_argsathermes_cli/config.py:5328and bridge parity tests intests/tools/test_terminal_config_env_sync.py.
Suggested changes
- Make the contract validator emit concrete warnings and tighten tests to assert those warnings, not just list shape.
- Move/update the custom-provider
max_tokensresolution so session metadata stays accurate. - Remove the binary artifact and the superseded docker bridge hunk.
Automated hermes-sweeper review.
| registered_keys = {b.config_key for b in CONFIG_BINDINGS} | ||
| _check_unregistered_keys(config, registered_keys, "", warnings) | ||
|
|
||
| return warnings |
There was a problem hiding this comment.
This function currently never appends to warnings, so gateway startup validation will always be silent even when a config binding is missing. The Phase 1 contract needs at least one concrete check that produces a warning and a test that asserts that warning.
| for key, value in config.items(): | ||
| full_key = f"{prefix}{key}" if not prefix else f"{prefix}.{key}" | ||
| if isinstance(value, dict) and key not in ("providers", "custom_providers", "models"): | ||
| _check_unregistered_keys(value, registered_keys, full_key, warnings) |
There was a problem hiding this comment.
strict=True still cannot report unregistered keys: full_key is computed but never compared to registered_keys or appended to warnings. Either implement the strict check here or remove the strict option/tests until it has behavior.
| custom_providers=_custom_providers, | ||
| ) | ||
| if _cp_max_tokens: | ||
| agent.max_tokens = int(_cp_max_tokens) |
There was a problem hiding this comment.
This updates agent.max_tokens after the session init model config has already been snapshotted on current main, so session DB metadata can still record max_tokens=None. Resolve this before the snapshot or update _session_init_model_config["max_tokens"] after setting it.
Summary
Implements Phase 1 of the Typed Config-Runtime Contract — a declarative config binding registry that catches orphan config fields at startup, before users discover them silently.
Along the way, fixes two concrete bugs that motivated this work.
Bug Fixes
Fix #28046:
max_tokenssilently ignored fromcustom_providersper-model configProblem: Only
context_lengthwas read fromcustom_providers[].models.<model>. Themax_tokensfield was silently ignored, always defaulting to 4096.Fix: Extract
get_custom_provider_model_field()as a generic lookup helper, addget_custom_provider_max_tokens()symmetric tocontext_length, and read it inagent_init.pyafter the existing context_length lookup.Fix #28863:
terminal.docker_extra_argssilently droppedProblem:
docker_extra_argswas declared inDEFAULT_CONFIGbut missing from the_terminal_env_mapbridge ingateway/run.py, so it was never passed to the terminal tool.Fix: Add the mapping entry.
Feature: Config-Runtime Contract Registry (Phase 1)
New module:
hermes_cli/config_contracts.pyCONFIG_BINDINGS— declarative registry of 29 config→runtime bindings (all terminal.* env vars, model.max_tokens, model.context_length, custom_providers wildcards)get_nested()— resolves dotted keys with wildcard support, including list-of-dicts (forcustom_providers)validate_config_bindings()— cross-checks loaded config against registry, returns warning listget_binding_report()— human-readable status report for debuggingGateway integration
At startup, after env var bridging, the gateway runs
validate_config_bindings()and logs any warnings. This is purely additive — warnings only, no behavior change, wrapped in try/except so it can never block startup.How this prevents future bugs
When a developer adds a new
terminal.*key toDEFAULT_CONFIG, the contract registry immediately shows it as unregistered. The next person who reviews the code sees the gap before shipping.Test Coverage
test_config_contracts.pytest_custom_provider_max_tokens.pytest_custom_provider_context_length.pyBackward Compatibility
get_custom_provider_context_length()signature unchangedget_custom_provider_max_tokens()is a new function (purely additive)config_contracts.pyis a new module (purely additive)Related
Files Changed
hermes_cli/config_contracts.pytests/hermes_cli/test_config_contracts.pytests/hermes_cli/test_custom_provider_max_tokens.pyagent/agent_init.pyhermes_cli/config.pygateway/run.py