fix(flows): scope the no-echo rule to values, keep the per-encounter no-op

Three points from review, all measured first:

- The comment justifying position-over-step-id claimed nothing echoed a
  nested caller key. Field names do: step_values.<step>.<field> goes into
  ignored_config_keys, which _unconsumed_reconfigure_keys unions into the
  reconfigure abort error — the same logged path. That is kept, because
  naming the field is the whole diagnostic for a typo and a name is not the
  secret a value is, so the comment is scoped to values instead.

- An empty object INSIDE a longer list is a capability, not a mistake: it
  means 'leave this encounter as it would be without the directive', which
  nothing else expresses — omitting the step affects every encounter, and
  {'field': None} is an explicit clear. Documented in the validator
  docstring and pinned by a test; entries that apply nothing anywhere are
  still rejected.

- context={} was a no-op (create_error_response gates on truthiness) and
  the guard reduced to . Both simplified.

Verified live on a real workday options flow before fixing: the [{}, ...]
shape is accepted, the empty entry is a genuine no-op, and the tail is
reported.

Found by Patch76 review on #2256.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr

Author: kingpanther13

src/ha_mcp/tools/config_entry_flow_form.py

@@ -852,6 +852,9 @@ def validate_step_values(config: dict[str, Any]) -> None:
 
     Accepted: a dict of ``step_id -> entry``, where an entry is a dict of field
     values, or a LIST of such dicts consumed one per encounter of that step.
+    An empty dict inside such a list is a per-encounter no-op — that encounter
+    behaves as it would with no directive at all — but an entry that applies
+    nothing anywhere is rejected.
     """
     # Key PRESENCE is the test, not truthiness: an explicit ``None`` is a
     # caller who meant to pass a directive and got the shape wrong, and the
@@ -883,12 +886,17 @@ def validate_step_values(config: dict[str, Any]) -> None:
             )
         )
 
-    # The step id is a caller-controlled key, and nothing echoed a NESTED one
-    # before this directive existed — the walker's own supplied_keys reports
-    # only top-level names. These errors reach the usage log unmasked (see the
-    # note above), so report the entry's POSITION instead: it is what the
-    # caller needs to find the entry in their own directive, and it cannot
-    # carry a value they typed (CodeRabbit review, issue #2254).
+    # Report the entry's POSITION, not the caller's step id: these errors reach
+    # the usage log unmasked (see the note above), and a step id is a
+    # caller-controlled key. The position still says which entry is wrong and
+    # cannot carry a value they typed (CodeRabbit review, issue #2254).
+    #
+    # Scoped to VALUES deliberately. Caller-supplied field NAMES do reach the
+    # same logged path — ``step_values.<step_id>.<field>`` goes into
+    # ``ignored_config_keys``, which ``_unconsumed_reconfigure_keys`` unions
+    # into the reconfigure abort error — and that is kept, because naming the
+    # field is the whole diagnostic for a typo and a name is not the secret a
+    # value is (Patch76 review, issue #2254).
     for index, entry in enumerate(directive.values(), start=1):
         where = f"{_PER_STEP_VALUES_KEY} entry #{index}"
         entries = entry if isinstance(entry, list) else [entry]
@@ -908,12 +916,18 @@ def validate_step_values(config: dict[str, Any]) -> None:
                     },
                 )
             )
-        # An entry that can apply nothing is a caller mistake in the same way a
-        # malformed one is, and it was reported inconsistently: a bare {} left
-        # a leftover the warning named, while [] and [{}] were popped at their
-        # first encounter and vanished, so the walk reported a clean success
-        # for a directive that did nothing (Patch76 review, issue #2254).
-        if not entries or not any(item for item in entries):
+        # An entry that can apply NOTHING AT ALL is a caller mistake in the
+        # same way a malformed one is, and it was reported inconsistently: a
+        # bare {} left a leftover the warning named, while [] and [{}] were
+        # popped at their first encounter and vanished, so the walk reported a
+        # clean success for a directive that did nothing (Patch76 review).
+        #
+        # An empty object INSIDE a longer list is deliberately not that: it
+        # means "leave this encounter as it would be without the directive",
+        # which nothing else expresses — omitting the step affects every
+        # encounter, and {"field": None} is an explicit clear rather than a
+        # fallback. So the test spans the whole entry rather than each item.
+        if not any(entries):
             raise_tool_error(
                 create_error_response(
                     ErrorCode.VALIDATION_INVALID_PARAMETER,
@@ -931,7 +945,6 @@ def validate_step_values(config: dict[str, Any]) -> None:
                 ErrorCode.VALIDATION_INVALID_PARAMETER,
                 f"'{_PER_STEP_VALUES_KEY}' is empty, so it would apply nothing",
                 suggestions=[f"Name a step: {_PER_STEP_VALUES_KEY}={example}."],
-                context={},
             )
         )
 

tests/src/unit/test_flow_options_preserve.py

@@ -1134,6 +1134,40 @@ def test_a_list_entry_can_clear_on_one_visit_only(self) -> None:
         # Dry: the step's own stored value takes over again.
         assert seen[2]["province"] == "BW"
 
+    def test_an_empty_object_in_a_list_is_a_per_encounter_no_op(self) -> None:
+        """The one empty shape that is a capability, not a mistake.
+
+        It means "leave this encounter as it would be without the directive",
+        which nothing else expresses: omitting the step affects EVERY
+        encounter, and {"field": None} is an explicit clear rather than a
+        fallback. The rejection deliberately spans the whole entry rather than
+        each item, so this survives while [] and [{}] do not (Patch76 review).
+        """
+        reuse_state = _ReuseState()
+        remaining: dict[str, Any] = {
+            "province": "BW",
+            "step_values": {"init": [{}, {"province": "NY"}]},
+        }
+        seen = [
+            _handle_form_step(
+                "flow-2254",
+                self._step("init"),
+                remaining,
+                None,
+                set(),
+                reuse_state,
+                keep_current_values=True,
+            )["province"]
+            for _ in range(2)
+        ]
+
+        # Encounter one falls back to what it would have been; two is addressed.
+        assert seen == ["BW", "NY"]
+        # ...while an entry that applies nothing anywhere is still rejected.
+        for dead in ([{}], [], {}, [{}, {}]):
+            with pytest.raises(ToolError):
+                validate_step_values({"step_values": {"init": dead}})
+
     def test_a_leftover_list_tail_is_reported(self) -> None:
         """A tail left over means the step ran fewer times than expected."""
         remaining: dict[str, Any] = {