refactor: source required_slots from INTENT-4 §10 manifest instead of __required_slots__

The orchestrator backstop for required_slots (PIPELINE-1 §6.2) was
reading from a made-up match_data.__required_slots__ convention that
no plugin implements. Per spec, required_slots is an optional field
in ovos.intent.register.template broadcasts (INTENT-4 §6.1), stored
in the orchestrator's manifest (INTENT-4 §10) as part of the full
registration payload.

- Remove __required_slots__ convention entirely
- _missing_required_slots now reads from IntentManifest.definition
  via _manifest_required_slots()
- Converted from staticmethod to instance method to access
  self.intent_manifest
- Tests rewritten to register template intents on the manifest
  rather than injecting __required_slots__ into match_data

Author: JarbasAl

ovos_core/intent_services/service.py

@@ -444,8 +444,7 @@ def _dispatch_match(self, match: IntentHandlerMatch, message: Message, lang: str
 
         return sess
 
-    @staticmethod
-    def _missing_required_slots(match: IntentHandlerMatch) -> List[str]:
+    def _missing_required_slots(self, match: IntentHandlerMatch) -> List[str]:
         """OVOS-PIPELINE-1 §6.2 orchestrator backstop for ``required_slots``.
 
         After a plugin returns a ``Match``, the orchestrator MUST verify that
@@ -462,39 +461,55 @@ def _missing_required_slots(match: IntentHandlerMatch) -> List[str]:
         Source of ``required_slots``: per §6.2 the orchestrator obtains this
         "from the same registration data the plugin consumed — in-process, this
         is available from the plugin's compiled state or from the orchestrator's
-        own manifest (INTENT-4 §10)". ovos-core does not yet maintain an INTENT-4
-        §10 registration manifest (no ``ovos.intent.list`` / ``ovos.intent.describe``
-        index is plumbed into the orchestrator), so the only conformant,
-        in-process channel a plugin has to surface the matched intent's
-        ``required_slots`` to the orchestrator is the ``Match`` itself. We read
-        it from ``match.match_data['__required_slots__']`` when present. The
-        captured slot map is ``match.match_data`` (OVOS-INTENT-3 §7; ``Match.slots``
-        in PIPELINE-1 §4.3), keyed by slot/vocabulary name.
-
-        A slot is "absent" when its key is missing from the slot map — a present
-        slot carrying a falsy value (e.g. ``0`` or ``False``) is a legitimate,
-        present slot and MUST NOT be filtered out.
-
-        When a plugin does not surface ``required_slots`` (the common case
-        today), this returns an empty list and the backstop is a no-op, leaving
-        engine-side enforcement authoritative.
-
-        TODO: once an INTENT-4 §10 manifest is plumbed into the orchestrator,
-        source ``required_slots`` from it (keyed on skill_id/intent_name/lang)
-        so the backstop covers every plugin regardless of whether it echoes the
-        constraint back in ``match_data``.
+        own manifest (INTENT-4 §10)". ovos-core now maintains an INTENT-4 §10
+        ``IntentManifest`` (``ovos.intent.list`` / ``ovos.intent.describe``)
+        plumbed into ``IntentService`` as ``self.intent_manifest``.
+
+        The manifest stores the full §5/§6 registration payload "as it was
+        broadcast" (§10.2). When a template intent registration (INTENT-4 §6.1)
+        carries an optional ``required_slots`` array, the manifest preserves it
+        in ``definition["required_slots"]``.  This method reads ``required_slots``
+        from the manifest entry matching ``skill_id`` / ``intent_name``.
+
+        A slot is "absent" when its key is missing from the slot map —
+        a present slot carrying a falsy value (e.g. ``0`` or ``False``)
+        is a legitimate, present slot and MUST NOT be filtered out.
+
+        When the intent has no ``required_slots`` declaration this returns
+        an empty list and the backstop is a no-op, leaving engine-side
+        enforcement authoritative.
 
         Returns:
             List[str]: required slot names absent from the match's slot map.
                 Empty when the match is conformant or carries no
                 ``required_slots``.
         """
-        match_data = match.match_data or {}
-        required_slots = match_data.get("__required_slots__")
+        required_slots = self._manifest_required_slots(match)
         if not required_slots:
             return []
+        match_data = match.match_data or {}
         return [slot for slot in required_slots if slot not in match_data]
 
+    def _manifest_required_slots(self, match: IntentHandlerMatch) -> List[str]:
+        """Look up ``required_slots`` from the INTENT-4 §10 manifest.
+
+        The manifest indexes every ``ovos.intent.register.template`` broadcast
+        (INTENT-4 §6.1), whose payload MAY carry an optional ``required_slots``
+        array.  This method retrieves it keyed by ``skill_id`` / ``intent_name``.
+
+        Returns:
+            List[str]: required slots from the manifest, or empty list.
+        """
+        if not self.intent_manifest or not match.skill_id or not match.match_type:
+            return []
+        intent_name = match.match_type.split(":")[-1]
+        for entry in self.intent_manifest._index.values():
+            if entry["skill_id"] == match.skill_id and entry["intent_name"] == intent_name:
+                req = (entry.get("definition") or {}).get("required_slots")
+                if req:
+                    return req
+        return []
+
     @staticmethod
     def _upload_match_data(utterance: str, intent: str, lang: str, match_data: dict):
         """if enabled upload the intent match data to a server, allowing users and developers

test/unittests/test_intent_service_extended.py

@@ -630,69 +630,113 @@ def fake_emit(m):
 class TestRequiredSlotsBackstop(unittest.TestCase):
     """OVOS-PIPELINE-1 §6.2 orchestrator backstop for required_slots."""
 
+    SKILL_ID = "music.skill"
+    INTENT_NAME = "play_music"
+    MATCH_TYPE = f"{SKILL_ID}:{INTENT_NAME}"
+
+    def setUp(self):
+        self.svc = _make_service()
+
+    def _register_template(self, skill_id=None, intent_name=None,
+                           required_slots=None):
+        """Register a template intent on the manifest (INTENT-4 §6.1)."""
+        msg = Message(
+            "ovos.intent.register.template",
+            {
+                "skill_id": skill_id or self.SKILL_ID,
+                "intent_name": intent_name or self.INTENT_NAME,
+                "lang": "en-US",
+                "samples": ["play {query}"],
+                "required_slots": required_slots or [],
+            },
+            {"skill_id": skill_id or self.SKILL_ID},
+        )
+        self.svc.intent_manifest._on_register(msg)
+
     @staticmethod
-    def _match_with_required(present_slots, required_slots):
-        data = {"skill_id": "music.skill"}
-        data.update(present_slots)
-        data["__required_slots__"] = required_slots
+    def _make_match(slot_map=None, skill_id=SKILL_ID, match_type=MATCH_TYPE):
+        """Create a match with given slot map (no __required_slots__)."""
+        data = {"skill_id": skill_id}
+        if slot_map:
+            data.update(slot_map)
         return IntentHandlerMatch(
-            match_type="music.skill:play_music",
+            match_type=match_type,
             match_data=data,
-            skill_id="music.skill",
+            skill_id=skill_id,
             utterance="play some jazz",
         )
 
     # --- _missing_required_slots unit ---------------------------------------
 
     def test_no_required_slots_is_noop(self):
-        """A match that carries no required_slots is never filtered."""
+        """No registered required_slots → backstop is a no-op."""
         match = _make_match()
-        self.assertEqual(IntentService._missing_required_slots(match), [])
+        self.assertEqual(self.svc._missing_required_slots(match), [])
 
     def test_all_required_present_passes(self):
-        match = self._match_with_required({"query": "some jazz"}, ["query"])
-        self.assertEqual(IntentService._missing_required_slots(match), [])
+        """All declared required_slots present in the slot map."""
+        self._register_template(required_slots=["query"])
+        match = self._make_match({"query": "some jazz"})
+        self.assertEqual(self.svc._missing_required_slots(match), [])
 
     def test_missing_required_slot_detected(self):
-        match = self._match_with_required({}, ["query"])
-        self.assertEqual(IntentService._missing_required_slots(match), ["query"])
+        """A required slot absent from the slot map is reported."""
+        self._register_template(required_slots=["query"])
+        match = self._make_match({})
+        self.assertEqual(self.svc._missing_required_slots(match), ["query"])
 
     def test_falsy_present_value_counts_as_present(self):
-        """Spec/CodeRabbit fix: a present slot carrying a falsy value (0/False/
-        empty string) is a legitimate, PRESENT slot and MUST NOT be filtered out —
-        only a truly-abSENT key is missing."""
+        """A falsy value (0/False/'') is still a PRESENT slot — only a missing
+        key is absent (INTENT-3 §5.3)."""
+        self._register_template(required_slots=["query"])
         for present in ("", 0, False):
             with self.subTest(present=present):
-                match = self._match_with_required({"query": present}, ["query"])
-                self.assertEqual(IntentService._missing_required_slots(match), [],
+                match = self._make_match({"query": present})
+                self.assertEqual(self.svc._missing_required_slots(match), [],
                                  f"falsy value {present!r} is present, not missing")
 
     def test_partial_required_slots_reported(self):
-        match = self._match_with_required({"query": "jazz"},
-                                          ["query", "engine"])
-        self.assertEqual(IntentService._missing_required_slots(match),
-                         ["engine"])
-
-    def test_none_match_data_is_safe(self):
-        match = IntentHandlerMatch(match_type="x:y", match_data=None,
-                                   skill_id="x")
-        self.assertEqual(IntentService._missing_required_slots(match), [])
+        """Only the slots missing from the slot map are reported."""
+        self._register_template(required_slots=["query", "engine"])
+        match = self._make_match({"query": "jazz"})
+        self.assertEqual(self.svc._missing_required_slots(match), ["engine"])
+
+    def test_none_match_data_reports_all_as_missing(self):
+        """match_data=None means no slots were extracted — every required slot
+        declared in the manifest is absent."""
+        self._register_template(skill_id="x", intent_name="y",
+                                required_slots=["q"])
+        match = IntentHandlerMatch(match_type="x:y", match_data=None, skill_id="x")
+        self.assertEqual(self.svc._missing_required_slots(match), ["q"])
+
+    def test_skill_with_no_manifest_entry_is_noop(self):
+        """A match from a skill whose intent is not in the manifest is not
+        filtered — no registration data, no required_slots to enforce."""
+        match = _make_match()
+        self.assertEqual(self.svc._missing_required_slots(match), [])
 
     # --- end-to-end match-loop behaviour ------------------------------------
 
-    def _run_loop(self, first_match):
+    def _run_loop(self, first_match, register_intent=True):
         """Drive handle_utterance with a single pipeline stage returning
         ``first_match``, then a second stage that always declines, and return
         whether the first match was dispatched and whether the no-match path
         was reached."""
         svc = _make_service()
+        if register_intent:
+            svc.intent_manifest._on_register(Message(
+                "ovos.intent.register.template",
+                {"skill_id": self.SKILL_ID,
+                 "intent_name": self.INTENT_NAME,
+                 "lang": "en-US",
+                 "samples": ["play {query}"],
+                 "required_slots": ["query"]},
+                {"skill_id": self.SKILL_ID},
+            ))
         sess = Session("s")
         emitted = []
         svc.bus.emit = lambda m: emitted.append(m)
         svc.send_complete_intent_failure = MagicMock()
-        # The §6.2 backstop runs strictly BEFORE _dispatch_match, so stub
-        # dispatch out: a match that survives the backstop reaches it, a filtered
-        # one never does.
         dispatched = []
 
         def fake_dispatch(match, message, lang, pipeline_id=None):
@@ -716,13 +760,13 @@ def test_match_missing_required_slot_is_skipped_to_no_match(self):
         """§6.2: a match missing a required slot is treated as if the plugin
         declined; iteration continues to no-match (send_complete_intent_failure
         emitted ovos.intent.unmatched)."""
-        match = self._match_with_required({}, ["query"])
+        match = self._make_match({})
         dispatched, _, unmatched = self._run_loop(match)
         self.assertFalse(dispatched, "match missing required_slots must not dispatch")
         self.assertTrue(unmatched, "missing required_slots must reach the no-match path")
 
     def test_match_with_all_required_slots_dispatches(self):
-        match = self._match_with_required({"query": "jazz"}, ["query"])
+        match = self._make_match({"query": "jazz"})
         dispatched, _, unmatched = self._run_loop(match)
         self.assertTrue(dispatched, "conformant match must dispatch")
         self.assertFalse(unmatched, "conformant match must not fall through to no-match")