Reject unknown config keys; stop mutating the input config during resolution#39
Merged
Conversation
Add an allowed-key check to validate_config so a misspelled top-level key (e.g. batery_kwh) raises listing the offending key instead of being silently dropped by merge_defaults and defaulting — a footgun for batch/parametric studies that returned plausible-but-wrong numbers. The allowed set is derived from DEFAULTS plus the required inputs and the optional montecarlo section, so it stays in sync as config keys are added. Also stop resolve_pv_system from writing the derived n_modules back into the merged config in place. It now returns the resolved count and resolve_app_config materialises it into a fresh dict, so the dict wrapped by the frozen ResolvedAppConfig is built once and the caller's input dict is left untouched. Targets the 0.3.1 roadmap config-validation items.
…idation # Conflicts: # CHANGELOG.md
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
1 participant
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
Two related config-layer hardening items from the 0.3.2 roadmap.
Reject unknown config keys
validate_confignow rejects unknown top-level keys. Previously a typo such asbatery_kwhslipped throughmerge_defaults({**DEFAULTS, **config}) and silently defaulted — e.g. the battery to0— so the run succeeded and returned plausible-but-wrong numbers with no warning. A real footgun for parametric/batch studies driven by many config files.It now raises listing the offending key(s), in the existing
"Unknown X. Available: ..."style. The allowed set is derived fromDEFAULTSplus the required inputs (location,annual_consumption_kwh,n_modules) and the optionalmontecarlosection (consumed by the Monte Carlo runner/CLI, which validates the same dict), so it stays in sync automatically as config keys are added.Stop mutating the input config during resolution
resolve_pv_systempreviously didcfg["n_modules"] = sum(...), writing into the same dict thefrozen=TrueResolvedAppConfigwraps — making the immutability partly cosmetic and the data flow harder to follow. It now returns the resolved module count, andresolve_app_configmaterialises it into a fresh dict, so the wrapped config is built once and the caller's input dict is left untouched.Tests
7 new tests: unknown-key rejection (including that the offending key is named), the
montecarlosection staying allowed, and that resolvingpv_arraysno longer mutates the caller's dict while still derivingn_modulescorrectly. Full suite green, ruff clean.Docs
Config reference note on strict key validation + CHANGELOG
[Unreleased].Note on overlap with #38
Both PRs touch
app_config.pyin non-overlapping regions and merge cleanly in either order. Because the allowed-key set is derived fromDEFAULTS, the new keys added in #38 are automatically accepted once both land.