feat: unify ha_config_set_helper response shape (closes #1293)

[Patch76]

What does this PR do?

Converges the four distinct response shapes that ha_config_set_helper exposed across create / update / _handle_flow_helper onto a single uniform contract.

Before (issue #1293 finding):

Action	Wrapper key	Warning placement
Create (simple)	"helper_data": {...}	nested helper_data["warning"] (singular string)
Update (simple/tag/fallback)	"updated_data": {...}	top-level response["warning"] (singular string)
Update (registry path)	"updated_data": {...}	nested updated_data["warning"] (singular string)
Flow helper	none — flat fields	top-level result["warnings"] (plural list)

A caller doing result.get("warning") found it on update (top-level singular) but missed every other case. A caller doing result.get("helper_data") missed update + flow.

After:

{
    "success": True,
    "action": "create" | "update",
    "helper_type": "...",
    "entity_id": "...",
    "data": { ... },              # was helper_data / updated_data
    "warnings": ["..."],          # always top-level list; omitted when empty
    "message": "...",
}

• data is the single wrapper key for the post-write payload across all three branches. Flow-helper's data contains {entry_id, title} (its HA flow_result payload); flat entry_id / title / entity_ids / applied etc. remain as convenience accessors — they're per-action metadata, not wrapper keys, and are heavily used by callers throughout the test suite.
• warnings is always a top-level list[str], omitted when empty. The four nested/singular shapes are gone.

Per .gemini/styleguide.md (lines 172-179): "Return value restructuring (as long as data still accessible) is NOT a breaking change — these are improvements - encourage them." Every previous value remains reachable at the new key.

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: 2447 passed, 1 skipped (numpy by-design), 0 failed
• New contract test tests/src/unit/test_helper_response_shape.py: 8/8 passed — locks the uniform shape across create + update + flow-helper, asserts warnings is always a list, asserts the legacy singular warning and the helper_data / updated_data wrappers are gone everywhere, and asserts a failed apply_entity_category surfaces the warning at the top level (never nested inside data)
• Mypy: clean on tools_config_helpers.py
• Ruff check: clean on all touched files

E2E tests (run in CI): 34 mechanical key-renames across 9 e2e test files updated to read from the new data key; flow-helper-flat-field accessors (entry_id, entity_ids, etc.) unchanged.

Future improvements

• Sibling-bug sweep (separate PR). The same result["warning"] = "..." singular-string anti-pattern lives in tools_config_scripts.py, tools_config_automations.py, tools_config_scenes.py, tools_groups.py, tools_service.py, tools_integrations.py, tools_energy.py, tools_system.py, tools_addons.py, tools_entities.py, tools_search.py, backup.py. Issue [BUG] Inconsistent response-shape across ha_config_set_helper actions (warning placement, data wrapper key) #1293 scopes only ha_config_set_helper; sweeping the rest belongs in a follow-up so the diff stays tractable.
• apply_entity_category category_warning lifted in helpers (this PR); still leaks in siblings. Both create + update branches in tools_config_helpers.py now route apply_entity_category through a temp dict and append any category_warning to the top-level warnings list, mirroring the precedent in _handle_flow_helper:L1403-1407. The same pattern still leaks nested category_warning in tools_config_scripts.py, tools_config_automations.py, tools_config_scenes.py; aligning those goes with the sibling-bug sweep above.
• Icon-propagation asymmetry on registry update. Both create + update branches now propagate area_id + labels into the response's data after a successful entity_registry/update, but neither propagates icon. Closing that gap is a small follow-up; out of scope here to keep the diff narrow.
• tests/src/e2e/ pre-existing ruff-format debt. The e2e test files I touched would reformat ~423 lines total under ruff format; this is master-state, not introduced by this PR. CI enforces only ruff check (verified in .github/workflows/pr.yml:45-46), so the debt is non-blocking and stays out-of-scope here.

Checklist

• I have updated documentation if needed (no public docs reference helper_data / updated_data as response key names — verified via grep on docs/, README.md, AGENTS.md, .gemini/styleguide.md)

[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 standardizes the response shape of the ha_config_set_helper tool to improve consistency and simplify client-side integration. By replacing multiple legacy response formats with a single, predictable contract, it ensures that callers can reliably access entity data and warnings regardless of the action performed. The changes maintain backward compatibility by ensuring all previous values remain reachable at the new keys.

Highlights

• Response Contract Unification: Converged four distinct response shapes from ha_config_set_helper into a single, uniform contract across create, update, and flow-helper actions.
• Standardized Payload Wrapper: Introduced a single 'data' wrapper key for post-write payloads, replacing inconsistent 'helper_data' and 'updated_data' keys.
• Warning Consolidation: Migrated all warning messages to a top-level 'warnings' list of strings, eliminating singular string-based warning fields.
• Comprehensive Test Updates: Updated E2E and unit tests to align with the new response structure and added new contract tests to prevent future regressions.

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 standardizes the response format for helper configuration tools—covering create, update, and flow-helper actions—by introducing a uniform "data" wrapper key and a top-level "warnings" list. These changes resolve inconsistencies where legacy keys like "helper_data", "updated_data", and singular "warning" strings were used across different code paths. Corresponding updates were applied to the E2E and unit test suites, including a new test file to lock in this contract. Feedback suggests ensuring the "tag" update branch also includes the "warnings" key to maintain total consistency with the other branches.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request standardizes the response structure for helper configuration tools by renaming legacy keys like helper_data and updated_data to a uniform data wrapper and consolidating warnings into a top-level warnings list. These changes are reflected across the codebase and verified with a new unit test suite. Feedback suggests further improving the uniform contract by ensuring that flow helper responses include metadata like area_id and labels within the data object, and addressing potential leaks of category_warning into the payload to maintain a clean and consistent API response across all logic branches.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request standardizes the response schema for helper configuration tools (Issue #1293) by renaming the "helper_data" and "updated_data" keys to a uniform "data" key and consolidating various warning formats into a top-level "warnings" list. These changes affect the create, update, and flow-helper branches in tools_config_helpers.py, with corresponding updates across the E2E and unit test suites. Review feedback suggests optimizing _handle_flow_helper by caching dictionary lookups and removing redundant .get() guards, as well as ensuring consistent key ordering in response dictionaries to maintain a stable API contract.

[Patch76]

Implementation Summary

Choices Made:

• Single data wrapper key + top-level warnings list — converges the four pre-[BUG] Inconsistent response-shape across ha_config_set_helper actions (warning placement, data wrapper key) #1293 shapes (nested helper_data["warning"], flat singular response["warning"], nested updated_data["warning"], flat plural result["warnings"]) onto one contract across create / update / flow-helper. Per .gemini/styleguide.md:172-179, return-value restructuring with data still reachable is explicitly NOT a breaking change.
• Flow-helper data = {entry_id, title} only — declined Gemini's L1578 finding that suggested folding area_id / labels / category in. Flow helpers can create N entities with potentially different per-entity outcomes; per-entity state lives in the flat applied[] accessor. Aggregating into data would lose that granularity.
• apply_entity_category routed through temp cat_result dict — adopted Gemini's L2692 + L3348 findings. On success data["category"] is set; on failure the message is appended to top-level warnings. Mirrors the precedent in _handle_flow_helper:L1403-1407. Replaces my initial PR-body's "needs signature change" justification — the temp-dict workaround is in-scope and addresses the helper path here (siblings still leak; tracked under follow-ups).
• Registry writes propagated into updated_data — adopted Gemini's L3348 sub-finding. After a successful entity_registry/update in the update branch, area_id + labels now flow into updated_data, matching the create branch at L2659-2666. Icon stays out for now to match the create branch's current behaviour (tracked under follow-ups).
• Tag-update branch follows the sibling pattern — adopted Gemini's earlier L2793 finding. Variable + conditional warnings mirrors create (L2697) and registry-update (L3346); no producer appends on the tag path today, but the conditional is in place for future ones.
• Final-wave cosmetic nits declined. The third Gemini review wave on 827407f raised three medium findings: caching flow_result["entry_id"] / ["title"] into local variables and switching from .get() to [] access (L1580), and dict-key-order on two data / entity_id returns (L2805, L3382). All three declined: none is backed by .gemini/styleguide.md, the .get() access pattern is consistent with the sibling line below (flow_result.get("message")), and dict key-order is cosmetic — callers reach for keys by name, not position. Held the line to avoid an N+1 review cycle on pure styling.

Problems Encountered:

• test_update_failed_category_apply_surfaces_top_level_warning initially errored on category-registry validation (category='cat-123' does not exist). The update path queries config/category_registry/list via _validate_registry_ids:L1069, and the generic endswith("/list") mock branch returned a list without cat-123 → fail-closed. Fixed in 827407f by adding an explicit config/category_registry/list handler in the test that includes the expected category_id. The create-side counterpart was unaffected because its single-value AsyncMock(return_value=...) returns a dict for every type, which fails the isinstance(inner, list) guard in _ws_list → fail-open.

Suggested Improvements (post-merge):

• Sibling-bug sweep. Same result["warning"] = "..." singular-string anti-pattern in 12 other tool files (listed in PR body); same nested category_warning pattern in tools_config_scripts.py / tools_config_automations.py / tools_config_scenes.py.
• Icon-propagation symmetry. Close the create+update gap above for icon (currently neither branch propagates it into the response's data).
• e2e ruff-format debt on touched e2e files (master-state, CI enforces only ruff check).

[kingpanther13]

Contract refactor is solid — cat_result temp-dict routing, the negative-invariant test, and the iterative tightening through commit 827407f all land cleanly. Items to fix before merge:

Requested changes

1. Missing unit coverage: registry-write-failure warning surfacing.

The diff edits the registry-write failure paths at tools_config_helpers.py:2670-2674 (create) and :3288-3290 (update) — both now warnings.append("Config updated but entity registry update failed: ..."). Neither path has a unit test. A regression re-nesting that string under data, or dropping the warnings.append entirely, wouldn't be caught.

A ~30-line mirror of test_create_failed_category_apply_surfaces_top_level_warning that patches the registry /update WS handler to return {"success": False, "error": "..."} and asserts both branches (create + update) surface the message at top level closes it.

2. Encode the contract in a TypedDict + builder, not just the test.

The contract currently lives in four return literals (:1571, :2680, :2796, :3357) plus _assert_uniform_shape. Your own comments at :2628 and :2755 ("No re-annotation — the create branch above already defined…") flag the parallel-construction drift risk in prose. A HelperResponse TypedDict + small _helper_response(action, helper_type, *, entity_id, data, warnings=None, **extras) builder collapses all four call sites onto one, naturally omits empty warnings, and converts the test-enforced invariant into a mypy-enforced one. Same surface area you're already touching — pull it in here rather than letting a fifth return literal accrete before someone else does it.

3. AGENTS.md "Return Values" section drift.

The documented degraded example still shows singular "warning": "...". Update to the plural-list contract this PR establishes so the styleguide doesn't contradict the merged code.

4. Stale line references in inline comments at :2681 and :3296 cite _handle_flow_helper (lines 1395-1407); the referenced precedent is now at :1400-1407. Update with the builder refactor.

5. Missing type annotation at :2798 — response = {...} literal lacks the explicit dict[str, Any] annotation its create-branch sibling at :2697 carries. The comment at :3375 claims "same binding"; the annotation makes that claim load-bearing. Moot if you do (2), but flagging in case you decline.

6. Logger asymmetry at :3286 — the update-branch registry failure does logger.warning(...) alongside warnings.append(...). The equivalent create-branch path at :2670 and the flow-helper path don't. Pick one stance for this PR's surface.