Reject unknown keys in config_overrides (extra="forbid" on BaseForecasterConfig)

[JackKelly]

🤖 This issue was written by Claude Code, acting on Jack's behalf.

BaseForecasterConfig is a plain pydantic BaseModel, so it inherits pydantic's default
extra="ignore". A config_overrides key that does not name a real field is silently dropped:

{"n_estimtors": 5000}

registers happily, trains with n_estimators=500 from the base YAML, and produces a run that
looks like a valid experiment.

Why this matters more than a typo usually would

Both planned search mechanisms are programmatic and unattended, so nobody reads the resolved
config before the runs go out:

• The LLM-driven auto-research agent registers experiments, materialises them and reads the leaderboard with no human in the loop.
• The training-history variant grid sweeps five dimensions at once.

One misspelled key in either produces a grid of identical runs that all score plausibly and all
land on the leaderboard. That is a principle 8
problem, not just an ergonomics one: the scores are real, but the experiments are not what they
claim to be, and nothing distinguishes the bad grid from a genuine null result.

It is also a naming-poka-yoke case — make the wrong usage fail to parse rather than merely be discouraged.

Proposed change

Add model_config = ConfigDict(extra="forbid") to BaseForecasterConfig
(packages/ml_core/src/ml_core/base_forecaster.py), so an unknown key raises ValidationError at
registration time, before any fold runs.

Check before landing: _resolve_forecaster_config pops both _target_ keys before construction,
so those must not trip the new strictness; and conf/model/xgboost.yaml's model_params must
contain no key that is not a declared field.

Out of scope

Not part of #228 — the
behaviour is identical before and after the Hydra removal (Hydra's instantiate also just called
the constructor). Filed separately so the removal stays a pure swap.

[JackKelly]

🤖 This comment was written by Claude Code, acting on Jack's behalf.

Two notes from #514, which
landed the Hydra removal and added a guard nearby.

extra="forbid" will make part of that guard redundant — decide deliberately whether to keep
it. _resolve_forecaster_config now rejects a config_overrides key that names one of the
model_params keys the resolver owns, via _UNOVERRIDABLE_MODEL_PARAMS in
src/nged_substation_forecast/defs/jobs.py. For _target_, extra="forbid" would catch the same
mistake on its own: the key is popped before construction, so an override of it arrives at the
constructor as an undeclared field and pydantic would raise ValidationError: Extra inputs are not permitted [_target_]. The explicit guard survives only on the strength of its message — it says
which key, why it cannot be set, and that base_model_config is the way to change the config
class — so keeping it is defensible, but it should be a conscious call rather than something left
in place by accident.

extra="forbid" does not cover experiment_name, so that half of the guard must stay.
experiment_name is a declared field on BaseForecasterConfig, not an extra. An override of it
validates cleanly and is then overwritten by the assignment that stamps the job's own
experiment_name — silently, and with no unknown key for pydantic to object to. This is the same
silent-discard failure the issue is about, arriving by a route the proposed change cannot close.

Also worth updating when this lands: the "Check before landing" paragraph above says the two
_target_ keys are popped before construction and must not trip the new strictness. That is still
true of the one in the YAML, but an override of _target_ is now refused earlier and never reaches
the constructor at all.