feat: mirror create-side validation guards onto update path (closes #1292)

[Patch76]

What does this PR do?

Closes the parity gap that issue #1292 surfaces: two input-side validation guards that fire on the create branch of ha_config_set_helper are absent from the update branch.

Helper type	Constraint	Create-side (existed since #1150)	Update-side (before this PR)
input_select	initial ∈ options	✓ enforced (src/ha_mcp/tools/tools_config_helpers.py:L2401-2422 on master)	✗ values merged, guard absent
input_datetime	at least one of has_date / has_time is True	✓ enforced (L2481-2488 on master)	✗ values merged, guard absent

A caller hitting either today reaches the same broken-entity state with success: true that #1150 closed for the create path, or surfaces the generic HA error message the create-side guard exists to pre-empt.

Approach — extract-and-share, matching the sibling pattern at tools_config_helpers.py:L869-998 (_validate_numeric_range, _validate_input_select_options, _validate_schedule_days):

• _validate_initial_in_options(options, initial) — accepts the resolved values from either branch; treats initial=None as the unset case so a merge-fall-through that leaves initial unset doesn't false-positive.
• _validate_datetime_has_date_or_time(has_date, has_time) — flags only the explicit (False, False) resolved-after-merge combo the issue targets; None passes through (not constrained).

Both validators are called from the create branch (replacing the inline blocks) and from the update branch after the merge step that resolves the final (options, initial) and (has_date, has_time) pairs.

Type of change

• 🐛 Bug fix
• ✨ New feature
• 📚 Documentation
• 🔧 Maintenance/refactor
• 🧪 Tests only
• 💥 Breaking change

Testing

• I have tested these changes with a LLM agent
• All automated tests pass (uv run pytest)
• Code follows style guidelines (uv run ruff check)

Local runs (Alpine, Python 3.13 musl):

• Unit tests: 2450 passed, 1 skipped (numpy by-design), 0 failed
• New tests (tests/src/unit/test_helper_input_validation.py): 11 added — 4 closing the previously-untested coverage gap for the [BUG] ha_config_set_helper: 19 bugs found in audit (silent drops, destructive UPDATE, broken multi-step flows, phantom-ID acceptance, etc.) #1150 create-side guards (the original PR didn't ship unit tests for them), 7 covering the new update-side parity:
  • TestInputSelectInitialInOptions (6 tests): create-side rejection + happy path; update-side rejection in all three merge-resolution combinations (caller-new-options + caller-new-initial; caller-only-options + existing-initial-falls-out; caller-only-initial + existing-options); update happy path.
  • TestInputDatetimeHasDateOrTime (5 tests): create-side (False, False) rejection + valid create; update-side rejection when caller disables both components or the only-remaining one; update happy path.
• Mypy: clean on tools_config_helpers.py
• Ruff check: clean on all touched files

Future improvements

• Sibling-validator audit on update paths: the same create-only-guard pattern that motivated this PR may exist elsewhere in tools_config_helpers.py (e.g. counter, timer, input_text ranges). The numeric-range guard already runs on update via _validate_numeric_range; the remaining per-type fields would benefit from the same audit. Out-of-scope here to keep the PR narrowly aligned with issue [FEATURE] Mirror create-side validation guards to update path in ha_config_set_helper (parity with #1150) #1292.

Checklist

• I have updated documentation if needed (no public docs reference these validators)

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a parity gap in input validation for 'input_select' and 'input_datetime' entities. Previously, certain validation constraints were only enforced during entity creation, allowing invalid configurations to be written during updates. By centralizing these checks into shared helper functions, the PR ensures that both create and update paths enforce the same data integrity rules, preventing the creation of broken entity states.

Highlights

• Validation Parity: Implemented shared validation guards for 'input_select' and 'input_datetime' types to ensure consistency between create and update operations.
• Refactored Logic: Extracted validation logic into reusable helper functions '_validate_initial_in_options' and '_validate_datetime_has_date_or_time' to eliminate code duplication.
• Enhanced Test Coverage: Added 11 new unit tests covering both the existing create-side guards and the newly implemented update-side parity checks.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors validation logic for input_select and input_datetime helpers into shared utility functions, ensuring consistent validation across both creation and update paths (addressing issue #1292). Specifically, it ensures initial values exist within the provided options and that input_datetime entities define at least a date or a time. Comprehensive unit tests were added to verify these guards. Feedback was provided to enhance the input_datetime error response by including relevant context and actionable suggestions in alignment with the repository's style guide.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors validation logic for input_select and input_datetime helpers into shared functions, extending validation to the update path to prevent broken entity configurations. It also adds comprehensive unit tests for these scenarios. Review feedback suggests adding defensive type checks for the options parameter and refining error message suggestions to be more accurate for update operations.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors validation logic for input_select and input_datetime helpers into shared utility functions and extends these guards to the update path in ha_config_set_helper to ensure parity with creation logic. New unit tests provide coverage for these validation scenarios. Feedback suggests improving the error response suggestions in _validate_initial_in_options to avoid misleading users during updates where configuration merging occurs, and to include a documentation link for consistency with sibling tools.

[Patch76]

Implementation Summary

Choices Made:

• Extract-and-share validators, mirror onto update path — _validate_initial_in_options(options, initial) and _validate_datetime_has_date_or_time(has_date, has_time) are reusable helpers called from both create (replacing the inline blocks the [BUG] ha_config_set_helper: 19 bugs found in audit (silent drops, destructive UPDATE, broken multi-step flows, phantom-ID acceptance, etc.) #1150 work shipped) and update (after the merge step resolves the final caller-or-existing values). Matches the established sibling pattern at tools_config_helpers.py:L869-998 for _validate_numeric_range, _validate_input_select_options, and _validate_schedule_days. Closes the parity gap that issue [FEATURE] Mirror create-side validation guards to update path in ha_config_set_helper (parity with #1150) #1292 surfaces: callers used to reach a broken-entity state with success: true on update for both input_select.initial ∉ options and input_datetime.(has_date, has_time) == (False, False).
• Datetime validator error response now structured per styleguide — adopted Gemini's first-wave finding on _validate_datetime_has_date_or_time. The error now ships context={has_date, has_time} so the failing payload is visible in the structured error, plus a two-entry suggestions array in the active-instruction style of _validate_initial_in_options. Documentation-link suggestion from the bot proposal dropped to keep parity with the sibling validators (neither _validate_initial_in_options nor _validate_input_select_options ships one).
• Defensive isinstance guard on _validate_initial_in_options — adopted Gemini's second-wave L878 finding. Mirrors the existing isinstance(options, list) shape guard in _validate_input_select_options:L932. Both current callers feed a list (create at L2460, update at L3091, the latter via the existing.get("options", []) default), so the guard is a no-op today; it prevents a future non-list caller from raising a confusing TypeError on initial not in options.
• Neutral suggestion wording on _validate_initial_in_options — adopted Gemini's second-wave L892 finding. The previous "Or omit initial so the entity starts unset." was accurate for create but misleading on the update path where the merge at L3079-3084 preserves the existing value when initial is None. Reworded to "Or omit initial to use the default or existing value." so the user-facing error covers both branches the validator is reachable from.
• Third-wave nit on the same suggestion declined. The L897 follow-up proposed a three-entry suggestions array including a Home Assistant documentation link and an explicit "new options may leave existing initial out" branch. Declined: sibling validators in the module (_validate_input_select_options, _validate_datetime_has_date_or_time after its first-wave fix) ship one suggestion without a doc-link, and the edge case is reachable from suggestion ci(deps): bump python-semantic-release/python-semantic-release from 9.10.0 to 10.4.1 in the actions group #1 ("Pick an initial value that's in options"). Held to the sibling pattern.

Problems Encountered:

• None during implementation. Local test loops stayed green across both review iterations.

Suggested Improvements (post-merge):

• Sibling-validator audit on other update paths. The same create-only-guard pattern that motivated this PR may exist elsewhere in tools_config_helpers.py (e.g. counter, timer, input_text ranges). The numeric-range guard already runs on update via _validate_numeric_range; the remaining per-type fields would benefit from the same audit. Out-of-scope here to keep the PR narrowly aligned with issue [FEATURE] Mirror create-side validation guards to update path in ha_config_set_helper (parity with #1150) #1292.

[kingpanther13]

Clean refactor — extract-and-share is faithful, merge ordering puts resolution before validation, ToolError propagation is intact through the outer guard at L3387. A few things to address before merge.

Requested changes

1. Strip issue-number refs from docstrings/comments. Per AGENTS.md "comments shouldn't reference the current task/fix/issue — belongs in the PR description." The #1150 / #1292 refs in _validate_initial_in_options (L867-885) and _validate_datetime_has_date_or_time (L887-898), in the call-site comments at L2459-2461 / L3087-3091 / L3187-3191, and in the test class docstrings at L181-187 / L305-311 will read as rot once the migration framing fades. The durable WHY is the parity invariant — keep that, drop the issue numbers.

2. Pick one shape strategy for options. _validate_initial_in_options declares options: list[Any] but the body still does isinstance(options, list). Sibling _validate_input_select_options at L869-908 uses options: Any + an internal shape check — match it, or trust the type and drop the runtime guard. The current "typed-narrowly but defended anyway" reads as confused intent.

3. The isinstance(options, list) guard is unreachable. Both call sites feed lists (caller path goes through parse_string_list_param; existing path defaults to []). If you keep the guard as defense-in-depth, add a direct unit test pinning the contract. Otherwise drop it — unreachable guards drift into "why is this here?" over time.

While you're in there

• Missing test edges. initial="" against options=["A","B"] (empty string treated as set, should reject), options=[] on update with any initial, and a happy-path update where caller passes has_date=None/has_time=None against an existing (False, True) config. Cheap to add and locks the contract.
• Hardcoded "input_select" in error context (_validate_initial_in_options L887). Fine today — both callers are input_select branches — but a future reuse on another helper type would emit misleading context. A helper_type param costs one line.
• Collapse _wire_existing_config into _wire_default_ws. It's a thin wrapper that differs only by passing existing_config=. Adding a default existing_config=None param to the original is one helper instead of two.
• PR description should note the create-branch suggestion text change. "Or omit initial so the entity starts unset" → "Or omit initial to use the default or existing value." Intentional generalization for shared use, but it's a user-visible message shift on the create path — worth surfacing so it doesn't get lost.