Skip to content

fix(schema): correct 5 default mismatches in param_definitions.json#830

Open
OptimalNothing90 wants to merge 1 commit into
davidusb-geek:masterfrom
OptimalNothing90:fix/param-definitions-default-mismatches
Open

fix(schema): correct 5 default mismatches in param_definitions.json#830
OptimalNothing90 wants to merge 1 commit into
davidusb-geek:masterfrom
OptimalNothing90:fix/param-definitions-default-mismatches

Conversation

@OptimalNothing90
Copy link
Copy Markdown

@OptimalNothing90 OptimalNothing90 commented Apr 29, 2026

Summary

Corrects 5 default-value / input-spelling mismatches in
param_definitions.json against config_defaults.json. Source-of-truth
(config_defaults.json) unchanged; only the GUI schema is brought back
in line so the web config form shows the actual runtime defaults.

Before / After

Key Field Before After Source
historic_days_to_retrieve default_value 2 9 config_defaults.json
historic_days_to_retrieve Description "...Defaults to 2" "...Defaults to 9" (text alignment)
inverter_ac_output_max default_value 0 5000 config_defaults.json
inverter_ac_input_max default_value 0 5000 config_defaults.json
load_forecast_method default_value "typical" "naive" config_defaults.json
ignore_pv_feedback_during_curtailment input "bool" "boolean" other booleans in file

Verification

Reproducer script + full audit:
https://github.com/OptimalNothing90/emhass-contributions/blob/master/audits/2026-04-28-param-definitions.md (Section 3)

Re-verified against current master tip on 2026-04-30 — all 5 still hold.

Notes

Summary by Sourcery

Align GUI parameter schema defaults and input types with the canonical runtime configuration defaults for affected parameters.

Bug Fixes:

  • Correct default values in the parameter schema for several configuration options to match config_defaults.json.
  • Fix the input type of ignore_pv_feedback_during_curtailment to use the proper boolean type in the schema.

Five entries in param_definitions.json (GUI hint metadata) disagreed
with config_defaults.json (authoritative runtime defaults). Schema
defaults dictate initial form values shown to users; mismatches mislead
new installations. Source-of-truth (config_defaults.json) unchanged.

- historic_days_to_retrieve: 2 → 9 (default_value + Description text)
- inverter_ac_output_max: 0 → 5000
- inverter_ac_input_max: 0 → 5000
- load_forecast_method: "typical" → "naive"
- ignore_pv_feedback_during_curtailment: input "bool" → "boolean"

Findings derived from input-side schema audit cross-checking
param_definitions.json against config_defaults.json and
utils.treat_runtimeparams. Audit + reproducer:
https://github.com/OptimalNothing90/emhass-contributions/blob/master/audits/2026-04-28-param-definitions.md
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Apr 29, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Aligns five GUI parameter schema defaults and one input type with the actual runtime defaults defined in config_defaults.json, so the web configuration form now reflects the true operational defaults without changing runtime behavior.

File-Level Changes

Change Details Files
Align historic_days_to_retrieve default and description with runtime default
  • Update default_value from 2 to 9 to match config_defaults.json
  • Update description text to state the default is 9 instead of 2
src/emhass/static/data/param_definitions.json
Fix inverter AC power limit parameter defaults
  • Update inverter_ac_output_max default_value from 0 to 5000 to match config_defaults.json
  • Update inverter_ac_input_max default_value from 0 to 5000 to match config_defaults.json
src/emhass/static/data/param_definitions.json
Correct load_forecast_method default to the actual runtime strategy
  • Update load_forecast_method default_value from "typical" to "naive" to match config_defaults.json
src/emhass/static/data/param_definitions.json
Normalize boolean input type spelling for ignore_pv_feedback_during_curtailment
  • Change input type string from "bool" to "boolean" to match other boolean parameters and schema conventions
src/emhass/static/data/param_definitions.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.46%. Comparing base (6537c47) to head (69522ed).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #830   +/-   ##
=======================================
  Coverage   82.46%   82.46%           
=======================================
  Files          10       10           
  Lines        5975     5975           
=======================================
  Hits         4927     4927           
  Misses       1048     1048           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davidusb-geek
Copy link
Copy Markdown
Owner

Hi, historic_days_to_retrieve should default to 2 (not 9 as proposed but yes there was a mismtach that needed fix).
load_forecast_method default should be "typical" (not "naive")
I can't remember why but I think that inverter_ac_output_max and inverter_ac_input_max should be dafaulted to 0, need to check this one

@OptimalNothing90
Copy link
Copy Markdown
Author

Can do, but this will change code default's as well.

For inverter, it seemed kind of philosophical to me, will change it too

@davidusb-geek
Copy link
Copy Markdown
Owner

davidusb-geek commented May 4, 2026

Can do, but this will change code default's as well.

For inverter, it seemed kind of philosophical to me, will change it too

@OptimalNothing90 So it seems that this PR's is updating param_definitions.json based on values from config_defaults.json. But it should be the other way around, update the config_defaults.json based on param_definitions.json.

@OptimalNothing90
Copy link
Copy Markdown
Author

@davidusb-geek I checked the code path before flipping. utils.py:1599-1638 in build_config_emhass loads config_defaults.json first, then user config.json overrides. param_definitions.json default_value is never read as a fallback in code — only displayed in the GUI configuration form. develop.md notes "should act as last-resort fallback if default_config.json is not found" but that path returns False today (line 1605).

So today there are two effective defaults per param:

  • Headless install (Docker + options.json, no GUI save): config_defaults wins
  • GUI install (user opens /configuration, saves): param_def value gets written into config.json and wins

That's the inconsistency. Direction matters because flipping changes behavior for Headless users:

Param config_defaults today param_def today Headless impact if flipped to param_def
historic_days_to_retrieve 9 2 ML forecaster trains on 7 fewer days
load_forecast_method "naive" "typical" switches default to ML-trained model for users who never set it
inverter_ac_output_max / _input_max 5000 0 optimizer treats as unconstrained (no cap)

The boolean fix (boolboolean for ignore_pv_feedback_during_curtailment) is unaffected — pure normalization to match the rest of the file.

Two options:

A) Treat config_defaults as source-of-truth (PR #830 as-is) — fix param_def to show what code does. Headless behavior unchanged. GUI users see the actual runtime values.

B) Treat param_def as source-of-truth — revert this PR's value changes, open follow-up PR updating config_defaults. Behavior changes for Headless users on those four params. Worth a heads-up in release notes.

Your call. Either way I'll keep the boolean type fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

3 participants