From 92b9bdc1ba631fc7e176e1f913a2478071d2e3f1 Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Sat, 11 Jul 2026 20:40:32 +0200 Subject: [PATCH] config: generalize config_validate into a shared schema engine require_schema(config, schema) is a generic recursive declarative checker (nested-dict schema, leaves name required type(s), each intermediate object node gets its own dict-type check so a wrong-typed section is named directly instead of surfacing as a confusing "missing child" error). validate_temp_ctrl() is now just require_schema(config, TEMPCTRL_SCHEMA) against a declarative schema dict, replacing the hand-rolled nested loops. utils/sud_validate.py's static structural check ('steps' must be a list) now goes through the same require_schema()/SUD_SCHEMA - one shared mechanism for both server config.json and sud.json documents. The genuinely dynamic per-step check (ramp.rate required only when a step sets its own temperature) stays hand-written, since it depends on the document's content, not just its shape. Added TestRequireSchema, exercising the generic engine directly against a schema unrelated to TempCtrl. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01F4oEE99rkKVU8L8tkzXWdG --- tests/utils/test_config_validate.py | 32 ++++++++++++++- utils/config_validate.py | 60 +++++++++++++++++++++++------ utils/sud_validate.py | 16 +++++++- 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/tests/utils/test_config_validate.py b/tests/utils/test_config_validate.py index 0be9b04..7e0e4b7 100644 --- a/tests/utils/test_config_validate.py +++ b/tests/utils/test_config_validate.py @@ -1,6 +1,6 @@ import unittest -from utils.config_validate import ConfigError, require, validate_temp_ctrl +from utils.config_validate import ConfigError, require, require_schema, validate_temp_ctrl def _valid_temp_ctrl_config(): @@ -42,6 +42,36 @@ class TestRequire(unittest.TestCase): self.assertEqual(require({"a": 1.5}, "a", type=(int, float)), 1.5) +class TestRequireSchema(unittest.TestCase): + """require_schema() is the generic engine both validate_temp_ctrl() + (config.json's TempCtrl section) and utils/sud_validate.py's + validate_sud() (a sud.json's fixed 'steps' shape) are built on - tested + here directly, against a schema unrelated to either, to confirm it's + not accidentally coupled to TempCtrl's specific shape.""" + + SCHEMA = {'a': {'b': int, 'c': (int, float)}, 'd': str} + + def test_valid_document_passes(self): + require_schema({'a': {'b': 1, 'c': 2.5}, 'd': 'x'}, self.SCHEMA) + + def test_missing_nested_leaf_raises(self): + with self.assertRaisesRegex(ConfigError, r'a\.b'): + require_schema({'a': {'c': 2.5}, 'd': 'x'}, self.SCHEMA) + + def test_missing_top_level_leaf_raises(self): + with self.assertRaisesRegex(ConfigError, r'missing required config key: d$'): + require_schema({'a': {'b': 1, 'c': 2.5}}, self.SCHEMA) + + def test_wrong_type_on_nested_object_itself_is_named_directly(self): + # 'a' present but not an object - must be reported as 'a' itself, + # not as a confusing "missing a.b"/"missing a.c". + with self.assertRaisesRegex(ConfigError, r'^config key a must be dict'): + require_schema({'a': 'not an object', 'd': 'x'}, self.SCHEMA) + + def test_extra_keys_not_in_schema_are_ignored(self): + require_schema({'a': {'b': 1, 'c': 2.5}, 'd': 'x', 'extra': 'stray'}, self.SCHEMA) + + class TestValidateTempCtrl(unittest.TestCase): def test_valid_config_passes(self): validate_temp_ctrl(_valid_temp_ctrl_config()) diff --git a/utils/config_validate.py b/utils/config_validate.py index c2603b8..289cb1c 100644 --- a/utils/config_validate.py +++ b/utils/config_validate.py @@ -6,6 +6,10 @@ class ConfigError(Exception): def require(config, path, type=None): + """Generic dotted-path lookup against any nested-dict document - not + tied to server config.json's own shape, so this is exactly as usable + for a sud.json schedule (see utils/sud_validate.py) as for TempCtrl + below.""" node = config parts = path.split('.') for i, key in enumerate(parts): @@ -18,18 +22,52 @@ def require(config, path, type=None): return node +def require_schema(config, schema, _prefix=''): + """Generic declarative counterpart to require(): schema mirrors the + document's own shape, with every leaf naming the required type(s) (see + TEMPCTRL_SCHEMA below for an example) - one shared mechanism for any + config-shaped document with a fixed structure, whether that's server + config.json's TempCtrl section or (for the parts of it that are static - + see utils/sud_validate.py for the parts that aren't, e.g. per-step + conditional requirements) a sud.json schedule. + + A schema value that's itself a dict describes a required sub-object: + checked (and reported, on failure, as its own dotted path) before + recursing into it, so a wrong-typed intermediate node (e.g. 'TempCtrl' + itself not being an object) is named directly rather than surfacing as + a confusing "missing" error on one of its children instead.""" + for key, spec in schema.items(): + path = '{}.{}'.format(_prefix, key) if _prefix else key + if isinstance(spec, dict): + require(config, path, type=dict) + require_schema(config, spec, path) + else: + require(config, path, type=spec) + + +_GAIN_SCHEMA = {'kp': (int, float), 'ki': (int, float), 'kd': (int, float), 'kt': (int, float)} + +# Only the gains every pid_type actually reads unconditionally are +# required; anything with its own default elsewhere (Outer.y_hold_min, +# Inner.Hold.yi_max, the whole Thresholds section - see +# temp_controller_base.py/temp_controller_fsm.py's DEFAULT_THRESHOLDS) +# stays optional here too, on purpose. +TEMPCTRL_SCHEMA = { + 'TempCtrl': { + 'pid_type': str, + 'Outer': _GAIN_SCHEMA, + 'Inner': { + 'Heat': _GAIN_SCHEMA, + 'Hold': _GAIN_SCHEMA, + 'Cool': _GAIN_SCHEMA, + }, + }, +} + + def validate_temp_ctrl(config): """Checked eagerly at startup so a missing/misnamed PID gain fails with a clear message here instead of a bare KeyError mid-__init__ or on the first process() tick (this bit us once already: config.json.sim's gain/ - Model nesting mismatch). Only the gains every pid_type actually reads - unconditionally are required; anything with its own default elsewhere - (Outer.y_hold_min, Inner.Hold.yi_max, the whole Thresholds section - - see temp_controller_base.py/temp_controller_fsm.py's DEFAULT_THRESHOLDS) - stays optional here too, on purpose.""" - require(config, 'TempCtrl', dict) - require(config, 'TempCtrl.pid_type', str) - for gain in ('kp', 'ki', 'kd', 'kt'): - require(config, 'TempCtrl.Outer.{}'.format(gain), (int, float)) - for branch in ('Heat', 'Hold', 'Cool'): - require(config, 'TempCtrl.Inner.{}.{}'.format(branch, gain), (int, float)) + Model nesting mismatch).""" + require_schema(config, TEMPCTRL_SCHEMA) diff --git a/utils/sud_validate.py b/utils/sud_validate.py index 8d202ad..b5db580 100644 --- a/utils/sud_validate.py +++ b/utils/sud_validate.py @@ -1,6 +1,18 @@ -from utils.config_validate import ConfigError, require +from utils.config_validate import ConfigError, require, require_schema from components.sud import Sud +# The only part of a sud.json document's shape that's fixed regardless of +# content - same declarative mechanism validate_temp_ctrl() uses for +# config.json's TempCtrl section (see config_validate.py's require_schema()). +# Everything else about a schedule (how many steps, which fields each one +# overrides vs. inherits from default.step, whether a step needs ramp.rate +# at all) depends on the document's own content, not just its shape, so it +# can't be expressed as a static schema - handled by the step loop below +# instead, still built on the same require()/ConfigError primitives. +SUD_SCHEMA = { + 'steps': list, +} + def validate_sud(data): """Fail-fast structural check for a sud.json document - same rationale @@ -15,7 +27,7 @@ def validate_sud(data): Reuses Sud._parse_data() itself (rather than re-implementing default- merging/step-building) so what's checked here can never drift from what actually gets parsed at runtime.""" - require(data, 'steps', type=list) + require_schema(data, SUD_SCHEMA) if not data['steps']: raise ConfigError("'steps' must be a non-empty list")