Skip to content

Commit 90a1fb4

Browse files
JarbasAlclaude
andcommitted
feat: OVOS-PIPELINE-1 §6.2 required_slots backstop + §7.3 reserved-name suppression
Reworked onto dev's IntentDispatcher (#788) architecture. The matched/unmatched/ handled emission this PR originally carried now lives in #788; the two deltas that remain unique are re-applied here: - §7.3: suppress the §7.1 `active_handlers` push for reserved-name dispatches (converse/response/stop/fallback/common_query) — a reserved name is a continuation/termination of an already-active skill, not a fresh activation. `activate_skill` is already a §7.1 shim over `add_active_handler`, so only the suppression gate was missing. - §6.2: orchestrator backstop that drops a match missing any `required_slots` (surfaced by the plugin in `match_data['__required_slots__']`) and continues iteration — a second line of defense behind engine-side enforcement. No-op and fully backward compatible when a plugin does not surface the constraint. Adds unit coverage for both the backstop and reserved-name suppression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 732f455 commit 90a1fb4

2 files changed

Lines changed: 147 additions & 1 deletion

File tree

ovos_core/intent_services/service.py

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,33 @@
6565

6666
_PIPELINE_RE = re.compile(r'-(high|medium|low)$')
6767

68+
# OVOS-PIPELINE-1 §7.3 reserved intent_names. A Match produced by one of the
69+
# reserving pipeline-plugin roles below is a reserved-name dispatch: §7.1
70+
# requires the ``session.active_handlers`` push to be SUPPRESSED for it, because
71+
# a reserved name represents a continuation/termination of an already-active
72+
# skill's participation, not a fresh activation. Keyed off the producing
73+
# pipeline_id (the role that holds the namespace lease), with the confidence
74+
# suffix (``-high``/``-medium``/``-low``) stripped before lookup.
75+
#
76+
# converse -> ovos-converse-pipeline-plugin (CONVERSE-1 §4/§5: converse, response)
77+
# stop -> ovos-stop-pipeline-plugin (STOP-1 §4: stop)
78+
# fallback -> ovos-fallback-pipeline-plugin (FALLBACK-1 §6.3: fallback)
79+
# common_query -> ovos-common-query-pipeline-plugin (COMMON-QUERY-1 §3: common_query)
80+
_RESERVED_NAME_PIPELINES = {
81+
"ovos-converse-pipeline-plugin",
82+
"ovos-stop-pipeline-plugin",
83+
"ovos-fallback-pipeline-plugin",
84+
"ovos-common-query-pipeline-plugin",
85+
}
86+
87+
88+
def _produces_reserved_name(pipeline_id: Optional[str]) -> bool:
89+
"""OVOS-PIPELINE-1 §7.3: True when ``pipeline_id`` is a reserved-name role
90+
whose dispatches must NOT stamp ``session.active_handlers`` (§7.1)."""
91+
if not pipeline_id:
92+
return False
93+
return _PIPELINE_RE.sub("", pipeline_id) in _RESERVED_NAME_PIPELINES
94+
6895

6996
def on_started():
7097
LOG.info('IntentService is starting up.')
@@ -293,6 +320,35 @@ def _emit_utterance_handled(self, dispatch_msg: Message):
293320
utterance."""
294321
self.bus.emit(dispatch_msg.forward(SpecMessage.UTTERANCE_HANDLED, {}))
295322

323+
@staticmethod
324+
def _missing_required_slots(match: IntentHandlerMatch) -> List[str]:
325+
"""OVOS-PIPELINE-1 §6.2 orchestrator backstop for ``required_slots``.
326+
327+
After a plugin returns a Match, the orchestrator verifies the match's
328+
slot map contains every slot the matched intent declares as required
329+
(OVOS-INTENT-3 §5.3, OVOS-INTENT-4 §6.1). If any is absent, the
330+
orchestrator treats the match as if the plugin had declined and
331+
continues iteration — no bus event is emitted; the only observable
332+
effect is a non-match (§6.2). The primary obligation to enforce
333+
``required_slots`` still lies with the engine during ``match()``; this
334+
is a second line of defense against engine bugs.
335+
336+
The plugin surfaces the constraint in ``match.match_data``: the required
337+
slot names under ``__required_slots__`` and the captured slot map as the
338+
remaining keys (OVOS-INTENT-3 §7; ``Match.slots`` in PIPELINE-1 §4.3).
339+
When a plugin does not surface it (the common case today) this returns
340+
an empty list and the backstop is a no-op, leaving engine-side
341+
enforcement authoritative — fully backward compatible.
342+
343+
Returns:
344+
List[str]: required slot names absent from the match's slot map.
345+
"""
346+
match_data = match.match_data or {}
347+
required_slots = match_data.get("__required_slots__")
348+
if not required_slots:
349+
return []
350+
return [slot for slot in required_slots if not match_data.get(slot)]
351+
296352
def _dispatch_match(self, match: IntentHandlerMatch, message: Message, lang: str,
297353
pipeline_id: str = None) -> None:
298354
"""Orchestrate the OVOS-PIPELINE-1 §6.1 post-match steps, then dispatch.
@@ -349,7 +405,15 @@ def _dispatch_match(self, match: IntentHandlerMatch, message: Message, lang: str
349405

350406
was_deactivated = match.skill_id in self._deactivations[sess.session_id]
351407
if not was_deactivated:
352-
sess.activate_skill(match.skill_id)
408+
# OVOS-PIPELINE-1 §7.1 pushes the skill onto the session's
409+
# active-handler recency list. §7.3 SUPPRESSES that push for
410+
# reserved intent_name dispatches (converse/response/stop/
411+
# fallback/common_query): a reserved name is a continuation
412+
# or termination of an already-active skill's participation,
413+
# not a fresh activation. `activate_skill` is a back-compat
414+
# shim over `add_active_handler` (§7.1) in current bus-client.
415+
if not _produces_reserved_name(pipeline_id):
416+
sess.activate_skill(match.skill_id)
353417
# emit event for skills callback -> self.handle_activate
354418
self.bus.emit(reply.forward(f"{match.skill_id}.activate"))
355419

@@ -520,6 +584,14 @@ def handle_utterance(self, message: Message):
520584
LOG.debug(
521585
f"ignoring match, intent '{match.match_type}' blacklisted by Session '{sess.session_id}'")
522586
continue
587+
# OVOS-PIPELINE-1 §6.2: if the matched intent is missing
588+
# any required slot, treat it as if the plugin had
589+
# declined and continue iteration; no bus event is emitted.
590+
missing = self._missing_required_slots(match)
591+
if missing:
592+
LOG.debug(f"ignoring match '{match.match_type}': "
593+
f"missing required slots {missing} (§6.2)")
594+
continue
523595
try:
524596
self._dispatch_match(match, message, intent_lang,
525597
pipeline_id=pipeline)

test/unittests/test_intent_service_extended.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,5 +602,79 @@ def test_shutdown_calls_pipeline_stop_and_shutdown(self):
602602
pipeline.shutdown.assert_called_once()
603603

604604

605+
# ---------------------------------------------------------------------------
606+
# OVOS-PIPELINE-1 §6.2 required_slots backstop
607+
# ---------------------------------------------------------------------------
608+
609+
class TestRequiredSlotsBackstop(unittest.TestCase):
610+
611+
def test_no_required_slots_is_noop(self):
612+
m = _make_match()
613+
m.match_data = {"skill_id": "s"}
614+
self.assertEqual(IntentService._missing_required_slots(m), [])
615+
616+
def test_all_required_slots_present(self):
617+
m = _make_match()
618+
m.match_data = {"__required_slots__": ["room"], "room": "kitchen"}
619+
self.assertEqual(IntentService._missing_required_slots(m), [])
620+
621+
def test_missing_required_slot_reported(self):
622+
m = _make_match()
623+
m.match_data = {"__required_slots__": ["room", "device"], "room": "kitchen"}
624+
self.assertEqual(IntentService._missing_required_slots(m), ["device"])
625+
626+
def test_falsy_slot_counts_as_missing(self):
627+
m = _make_match()
628+
m.match_data = {"__required_slots__": ["room"], "room": ""}
629+
self.assertEqual(IntentService._missing_required_slots(m), ["room"])
630+
631+
632+
# ---------------------------------------------------------------------------
633+
# OVOS-PIPELINE-1 §7.1/§7.3 active-handler push + reserved-name suppression
634+
# ---------------------------------------------------------------------------
635+
636+
class TestReservedNameActivation(unittest.TestCase):
637+
638+
def _dispatch(self, pipeline_id):
639+
svc = _make_service()
640+
sess = Session("s1")
641+
msg = Message("recognizer_loop:utterance", {"utterances": ["hi"]},
642+
{"session": sess.serialize()})
643+
match = _make_match(match_type="test.skill:intent",
644+
skill_id="test.skill", session=sess)
645+
svc._dispatch_match(match, msg, "en-US", pipeline_id=pipeline_id)
646+
return sess
647+
648+
def test_regular_pipeline_pushes_active_handler(self):
649+
sess = self._dispatch("ovos-adapt-pipeline-plugin-high")
650+
ids = [h.get("skill_id") if isinstance(h, dict) else getattr(h, "skill_id", h)
651+
for h in sess.active_handlers]
652+
self.assertIn("test.skill", ids)
653+
654+
def test_reserved_name_pipeline_suppresses_push(self):
655+
# §7.3: converse/stop/fallback/common_query dispatches must NOT push
656+
for pid in ("ovos-converse-pipeline-plugin",
657+
"ovos-stop-pipeline-plugin-high",
658+
"ovos-fallback-pipeline-plugin-medium",
659+
"ovos-common-query-pipeline-plugin"):
660+
sess = self._dispatch(pid)
661+
ids = [h.get("skill_id") if isinstance(h, dict) else getattr(h, "skill_id", h)
662+
for h in sess.active_handlers]
663+
self.assertNotIn("test.skill", ids, f"{pid} should suppress the push")
664+
665+
666+
class TestProducesReservedName(unittest.TestCase):
667+
668+
def test_reserved_roles_true_with_confidence_suffix(self):
669+
from ovos_core.intent_services.service import _produces_reserved_name
670+
self.assertTrue(_produces_reserved_name("ovos-stop-pipeline-plugin-high"))
671+
self.assertTrue(_produces_reserved_name("ovos-converse-pipeline-plugin"))
672+
673+
def test_regular_role_false(self):
674+
from ovos_core.intent_services.service import _produces_reserved_name
675+
self.assertFalse(_produces_reserved_name("ovos-adapt-pipeline-plugin-high"))
676+
self.assertFalse(_produces_reserved_name(None))
677+
678+
605679
if __name__ == "__main__":
606680
unittest.main()

0 commit comments

Comments
 (0)