Skip to content

Sentence trigger - #869

Open
contagon wants to merge 10 commits into
custom-components:masterfrom
contagon:sentence_trigger
Open

Sentence trigger#869
contagon wants to merge 10 commits into
custom-components:masterfrom
contagon:sentence_trigger

Conversation

@contagon

@contagon contagon commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

This is the first of two PRs for integrating pyscript with HA assist pipelines. It ties into the "exact matches" portion of the default HA assist pipeline. Should close #672.

spoken response -> exact sentence check -> LLM -> tool calling

This PR adds in sentence_trigger that (should) work near identically to the built-in sentence trigger. This includes return values being spoken, wildcards using {}, one of the following using (a|b|c), and optional words in []. I've been using this for ~week now and it's been working fairly flawless on basic cases.

Let me know if there's anything that needs tweaking and as always I recommend giving it a shot first before merging. Next I hope to play with a @intent decorator that would register a method as an intent/tool for agents to call, similar to this integration.

(Also shoutout to @dmamelin, the new decorator registry made this a breeze!)

@dmamelin

dmamelin commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Nice implementation! I still need more time for a full review.

My first thought is to pass slots as named function arguments. That would be much more convenient.

#From: 
@sentence_trigger("turn on {name} in the {area}")
  def voice_on(sentence, slots):
      service.call("light", "turn_on", entity_id=f"light.{slots['name']}")
      return f"Turned on {slots['name']} in the {slots['area']}"
      
#To:
@sentence_trigger("turn on {name} in the {area}")
def voice_on(sentence, name, area):
      service.call("light", "turn_on", entity_id=f"light.{name}")
      return f"Turned on {name} in the {area}"
  

Any objections?

Edit:
Oh. This is blocked by TRIGGER_KWARGS.

In my head, the legacy decorators and the new DM are already decoupled. I will try to speed up the work on removing the old subsystem.

Alternatively, I can make a PR where DM is not tied to TRIGGER_KWARGS.

@contagon

contagon commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

We keep doing things simultaneously! I just came to the same conclusion.

So I think the options are,

  1. Leave as is with slots: dict as an argument
  2. Switch, but accept missing slot arguments as invalid (which might be ok? At least temporarily?).
@sentence_trigger("turn on {name} in the {area}")
def voice_on(sentence, name): # missing area!
      service.call("light", "turn_on", entity_id=f"light.{name}")
      return f"Turned on {name}"

If all slots are included, this would work fine with the current TRIGGER_KWARGS checks.

  1. Rework things internally before this PR lands.

I definitely prefer either 2 or 3 as the long term solution, but am open to either option.

@ALERTua

ALERTua commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

I have a few spare tokens, and there can be no such thing as an unneeded review, so here's what Opus said:


Reviewed at 19d0da6 against HA dev. I read the code, the tests, the docs and the
relevant Home Assistant sources, but I did not run the test suite.

This is clean work, and the test file covers a lot: validators, apostrophes, positional and
list sentences, unregister on reload, old-HA fallback. I checked the two HA-facing claims and
both hold: AgentManager.register_trigger(sentences, trigger_callback) matches the call, and
a None return really does produce the translated "Done" response (default_agent.py, the
response_set_by_trigger block), so the timeout paragraph in the docs is accurate.

Six points below.

1. An exception or a cancellation makes the caller wait for the whole timeout

SentenceTriggerDecorator implements handle_call_result only. @webhook_handler, which this
code otherwise follows closely, also implements handle_call_exception and
handle_call_canceled, and both resolve the same future.

Without them, the future stays pending in three reachable cases:

  • the decorated function raises;
  • a CallHandlerDecorator returns False, for example @task_unique killing the previous run;
  • a TriggerHandlerDecorator returns False, for example @state_active.

In each case _trigger_callback sits in asyncio.wait_for until timeout expires, so the
speaker stays silent for 10 seconds and then says "Done".

Your own test shows the cost: test_sentence_trigger_exception_returns_none does not pass a
timeout, so it asserts the correct result only after ten real seconds. Adding the two
handlers fixes the user-facing delay and makes that test instant.

2. task.wait_until(sentence_trigger=...) is now accepted, and the voice side hangs

DecoratorRegistry.wait_until() builds a decorator for every registered TriggerDecorator
whose name appears in the call, so registering sentence_trigger makes
task.wait_until(sentence_trigger="...") work automatically. WaitUntilDecoratorManager
resolves its own future and never calls the function, so handle_call_result never runs and
the conversation waits out the full timeout before falling back to "Done".

@webhook_handler has the same gap today, so this may be acceptable for now. Either way it
deserves a decision: document sentence_trigger in the task.wait_until argument list in
docs/reference.rst, or reject it there explicitly.

3. A 10 second default is long for a spoken interaction

HA awaits the trigger callbacks inside the conversation pipeline, so this timeout is dead air
for the person talking. 10 seconds is a reasonable default for @webhook_handler over HTTP,
but for voice a lower value (2 to 5 seconds) fails faster and still covers a normal script.
With point 1 unfixed, every raised exception costs the full 10 seconds.

Also, vol.Range(min=0) accepts timeout=0, which always times out immediately. min above
zero would reject that at validation time.

4. start() swallows every exception, and the trigger then stays silently dead

except Exception as err:
    _LOGGER.warning("sentence_trigger %s failed to register; conversations unavailable: %s", ...)
    return

This catches a genuine bug in pyscript exactly like a missing component, and the message lands
in the integration log rather than in the user's pyscript log, where the rest of the decorator
errors go. Catching ImportError and AttributeError (as _import_validators already does),
and reporting through self.dm, keeps real errors visible.

For the version context: I checked HA releases, and AgentManager.register_trigger and
ConversationInput.satellite_id both appear in 2025.10.0 and are absent in 2025.8.0. So on
older HA this path is the only thing that runs, and the decorator is permanently inert with one
warning line. Since both landed together, reading user_input.satellite_id unguarded is safe.

5. _unregister has no class-level default

_unregister: CALLBACK_TYPE | None is an annotation only, and the value is assigned in
validate(). If stop() runs before a successful validate(), if self._unregister raises
AttributeError. EventTriggerDecorator avoids this with a real default
(remove_listener_callback: CALLBACK_TYPE | None = None); the same one-line change works here,
and the assignment in validate() can go.

6. str(response) speaks a Python repr for a non-string return

A function that returns a dict or a list gets str() applied, so the speaker reads
{'a': 1} out loud. @webhook_handler handles this with _to_response and logs a warning for
an unsupported type. A warning plus "Done" is friendlier than reading a repr aloud.

On passing slots as named arguments

About the idea in the thread: once #870 lands, def voice_on(name, area) becomes possible, and
it does read much better. One argument for keeping slots and details as well: slot names
come from user templates, so a template like "set the timer to {value}" produces a slot named
value, which collides with the state-trigger context name. The nested dict has no such
collision, so keeping it alongside the named arguments avoids a class of confusing overwrites.

Minor

  • The comment # is_valid_setence only available after HA 2026.7 has a typo in the name. The
    version claim is correct: is_valid_sentence is in 2026.7.0 and absent in 2026.6.0.
  • The eval.py TRIGGER_KWARGS hunk becomes unnecessary once Decouple DecoratorManager from eval.py TRIGGER_KWARGS allowlist #870 lands, so it is worth a
    follow-up note. Meanwhile it adds generic names such as device_id to a global allowlist,
    which weakens the unexpected-keyword check for every other function, @service included.
  • .gitignore now ends without a newline.
  • The "not a valid option" message match in decorator_abc.py is a string match against a
    voluptuous message, which will drift again on the next wording change. It also affects all
    decorators, so it may belong in its own PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Sentence Trigger

3 participants