Skip to content

arch: unify Battery/Home settings update paths to eliminate *_STORE_TO_API translation registries #219

Description

@johanzander

Background

Follow-up to #197 (Price settings, fixed in #216). #197 identified that PriceSettings.update() was designed to accept camelCase payloads via a hand-maintained PRICE_STORE_TO_API registry, while the PATCH path already bypassed it and passed snake_case directly — working only because _camel_to_snake() is a no-op on snake_case input. #216 unified Price's paths but intentionally left Battery and Home untouched to keep that PR scoped to its title.

The problem

Same dual-path pattern, verified against current main:

  • BATTERY_STORE_TO_API (backend/api_conversion.py:24) and HOME_STORE_TO_API (backend/api_conversion.py:34) are hand-maintained snake_case→camelCase registries used only by the startup path (build_system_settings()).
  • The PATCH handler already passes raw snake_case store dicts straight to update_settings() (backend/api.py:262 for battery, :272 for home), bypassing both registries entirely.
  • BatterySettings.update() and HomeSettings.update() (core/bess/settings.py:141, :196) still call _camel_to_snake() on every kwarg — a no-op on already-snake_case keys, making the "camelCase" contract fictional for the PATCH path.

Why this is fragile

Identical to #197's analysis:

  1. A field added to BatterySettings/HomeSettings without a matching registry entry silently vanishes on restart (startup path) while continuing to work via PATCH (session-only) — the exact bug class behind Belpex H integration? #126.
  2. TestBatteryModelAttrsConsistency (backend/tests/test_settings_contracts.py:246) exists as a structural guard for Battery, but there is no equivalent for Home, and neither has the startup/PATCH round-trip integration test that fix: unify Battery/Home/Price settings startup/PATCH paths, fix live efficiency-field bug (#197) #216 added for Price (TestPriceSettingsRoundTrip).
  3. "Works in session, breaks on restart" is undetectable by manual testing, since developers don't restart the service during dev.

Proposed fix

Apply the same pattern #216 used for Price:

  1. core/bess/settings.py: BatterySettings.update() and HomeSettings.update() drop the _camel_to_snake() call — kwargs must already be snake_case.
  2. backend/api_conversion.py: replace BATTERY_STORE_TO_API / HOME_STORE_TO_API translation dicts with presence-validation-only BATTERY_REQUIRED_FIELDS / HOME_REQUIRED_FIELDS frozensets; build_system_settings() passes the battery/home sections through unchanged instead of translating keys.
  3. backend/api.py: audit for any remaining camelCase producer feeding battery/home live-apply paths (mirroring the setup_complete fix in fix: unify Battery/Home/Price settings startup/PATCH paths, fix live efficiency-field bug (#197) #216) and convert to snake_case if found.
  4. backend/tests/test_settings_contracts.py: add TestHomeModelAttrsConsistency (Battery's equivalent already exists) plus TestBatterySettingsRoundTrip and TestHomeSettingsRoundTrip.
  5. core/bess/tests/unit/test_settings.py: add direct unit coverage for BatterySettings.update() / HomeSettings.update()'s new snake_case-only contract.

Files affected

  • core/bess/settings.py
  • backend/api_conversion.py
  • backend/api.py
  • backend/tests/test_settings_contracts.py
  • core/bess/tests/unit/test_settings.py

Acceptance criteria

Metadata

Metadata

Assignees

No one assigned

    Labels

    bot-analyzedTriage bot has processed this issueenhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions