fix: expand inline <voc> refs before registering intents to the engine

ovos_workshop/intents.py

@@ -11,7 +11,7 @@
 # their long-standing `from ovos_workshop.intents import IntentBuilder` import while
 # the single source of truth is the spec. The `IntentServiceInterface` producer
 # (and the munge_* helpers) below consume these definitions.
-from ovos_spec_tools import Intent, IntentBuilder, open_intent_envelope
+from ovos_spec_tools import Intent, IntentBuilder, inline_keywords, open_intent_envelope
 
 
 def to_alnum(skill_id: str) -> str:
@@ -271,7 +271,8 @@ def remove_adapt_context(self, context: str):
 
     def register_padatious_intent(self, intent_name: str, filename: str,
                                   lang: str, string_blacklist: Optional[List[str]] = None,
-                                  slot_blacklist: Optional[Dict[str, List[str]]] = None):
+                                  slot_blacklist: Optional[Dict[str, List[str]]] = None,
+                                  vocabs: Optional[Dict[str, List[str]]] = None):
         """
         Register a Padatious intent file with the intent service.
         @param intent_name: Unique intent identifier
@@ -283,6 +284,11 @@ def register_padatious_intent(self, intent_name: str, filename: str,
         @param slot_blacklist: OVOS-INTENT-2 §4.3 slot-value exclusions keyed by
             slot name, ``{slot: [excluded phrases]}`` — values that MUST NOT
             fill the named ``{slot}`` (each slot's sibling `<slot>.blacklist`).
+        @param vocabs: ``{name: members}`` of the sibling vocabularies, so an
+            inline ``<name>`` reference (OVOS-INTENT-1 §3.7) is baked into the
+            samples as an ``(a|b|c)`` group before they reach the engine — the
+            padatious/padacioso bus protocol carries only samples, never the
+            ``.voc`` content, so the reference must be resolved here.
         """
         if not isinstance(filename, str):
             raise ValueError('Filename path must be a string')
@@ -291,6 +297,9 @@ def register_padatious_intent(self, intent_name: str, filename: str,
         with open(filename) as f:
             samples = [_ for _ in f.read().split("\n") if _
                        and not _.startswith("#")]
+        if vocabs:
+            # OVOS-INTENT-1 §3.7: resolve every inline <name> in place
+            samples = [inline_keywords(s, vocabs) for s in samples]
         data = {'file_name': filename,
                 "samples": samples,
                 'name': intent_name,

ovos_workshop/resource_files.py

@@ -728,11 +728,13 @@ def _locale_vocabularies(self) -> Dict[str, List[str]]:
                     name = file_name[:-len(".voc")].lower()
                     if name in vocabularies:
                         continue
-                    members = []
+                    members: List[str] = []
                     with open(Path(directory, file_name)) as voc_file:
-                        for line in (ln.strip() for ln in voc_file):
+                        for line in (ln.strip().lower() for ln in voc_file):
                             if line and not line.startswith("#"):
-                                members.append(line.lower())
+                                # a .voc line MAY hold comma/parenthesis
+                                # alternates; every phrasing is a member
+                                members.extend(flatten_list(expand_template(line)))
                     if members:
                         vocabularies[name] = members
         return vocabularies
@@ -896,6 +898,20 @@ def load_skill_vocabulary(self, alphanumeric_skill_id: str) -> dict:
 
         return skill_vocabulary
 
+    def vocabularies(self) -> Dict[str, List[str]]:
+        """Build a ``{name: members}`` map from every ``.voc`` in this
+        language's locale tree.
+
+        Feeds the inline ``<name>`` resolution required by OVOS-INTENT-1 §3.7:
+        a reference in an ``.intent`` (or ``.blacklist``) expands in place from
+        the sibling vocabulary of that name. Padatious and padacioso never read
+        ``.voc`` files at match time, so the reference must be baked into the
+        template before it reaches the engine. User overrides take precedence
+        over the skill's own resources, which take precedence over
+        workshop-shipped resources.
+        """
+        return self._locale_vocabularies()
+
     def load_skill_regex(self, alphanumeric_skill_id: str) -> List[str]:
         """
         Load all regex files in the skill's resources

ovos_workshop/skills/ovos.py

@@ -1365,9 +1365,12 @@ def register_intent_file(self, intent_file: str, handler: callable,
                 if phrases:
                     slot_blacklist[slot] = phrases
 
+            # OVOS-INTENT-1 §3.7: supply the sibling vocabularies so an inline
+            # <name> reference in the .intent is baked into the samples before
+            # they are sent to the engine over the bus
             self.intent_service.register_padatious_intent(
                 name, filename, lang, string_blacklist=disallowed_strings,
-                slot_blacklist=slot_blacklist)
+                slot_blacklist=slot_blacklist, vocabs=resources.vocabularies())
         if handler:
             self.add_event(name, handler, 'mycroft.skill.handler',
                            activation=True, is_intent=True)

test/unittests/skills/test_base.py

@@ -19,7 +19,7 @@
 from logging import Logger
 from threading import Event, Thread
 from time import time
-from unittest.mock import Mock
+from unittest.mock import ANY, Mock
 from os.path import join, dirname, isdir
 from ovos_workshop.skills.ovos import OVOSSkill
 
@@ -443,7 +443,7 @@ def test_register_intent_file(self):
         skill.config_core["secondary_langs"] = []
         skill.register_intent_file("time.intent", Mock(__name__="test"))
         skill.intent_service.register_padatious_intent.assert_called_once_with(
-            f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={})
+            f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={}, vocabs=ANY)
 
         # With secondary language
         skill.intent_service.register_padatious_intent.reset_mock()
@@ -452,9 +452,9 @@ def test_register_intent_file(self):
         self.assertEqual(
             skill.intent_service.register_padatious_intent.call_count, 2)
         skill.intent_service.register_padatious_intent.assert_any_call(
-            f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={})
+            f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={}, vocabs=ANY)
         skill.intent_service.register_padatious_intent.assert_any_call(
-            f"{skill.skill_id}:time.intent", uk_intent_file, "uk-UA", string_blacklist=[], slot_blacklist={})
+            f"{skill.skill_id}:time.intent", uk_intent_file, "uk-UA", string_blacklist=[], slot_blacklist={}, vocabs=ANY)
 
     def test_register_entity_file(self):
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)

test/unittests/test_inline_vocab_refs.py

@@ -1,12 +1,22 @@
+"""OVOS-INTENT-1 §3.7: an inline ``<name>`` reference in a ``.intent`` file is
+an authoring convenience that must expand in place from the sibling ``.voc`` of
+that name before the template reaches an intent engine. The padatious/padacioso
+bus protocol carries only samples — never the ``.voc`` content — so the
+reference has to be resolved during registration, otherwise the raw ``<name>``
+token is trained into the engine and the intent never matches."""
 import unittest
-from tempfile import TemporaryDirectory
 from pathlib import Path
+from tempfile import TemporaryDirectory
+
+from ovos_utils.fakebus import FakeBus
+from ovos_workshop.intents import IntentServiceInterface
+from ovos_workshop.resource_files import ResourceFile, SkillResources
 
 
 class TestInlineVocabReferences(unittest.TestCase):
-    """OVOS-INTENT-1 §3.7: an inline ``<name>`` reference in a ``.intent`` or
-    ``.blacklist`` file must expand in place from the sibling ``.voc`` of that
-    name found in the same locale directory."""
+    """The load-time resolution path: ``load_intent_file`` / ``load_blacklist_file``
+    expand every inline ``<name>`` from the sibling ``.voc`` of that name found in
+    the same locale directory."""
 
     def _skill_dir(self, files: dict) -> str:
         tmp = TemporaryDirectory()
@@ -18,7 +28,6 @@ def _skill_dir(self, files: dict) -> str:
         return tmp.name
 
     def _resources(self, skill_dir):
-        from ovos_workshop.resource_files import SkillResources
         return SkillResources(skill_dir, "en-us", skill_id="test.skill")
 
     def test_intent_inline_vocab(self):
@@ -48,5 +57,114 @@ def test_intent_without_inline_ref_unchanged(self):
         self.assertIn("turn on the light", intents)
 
 
+class TestInlineVocabResources(unittest.TestCase):
+    def _resources(self, files):
+        tmp = TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        locale = Path(tmp.name, "locale", "en-us")
+        locale.mkdir(parents=True)
+        for name, content in files.items():
+            (locale / name).write_text(content)
+        return SkillResources(tmp.name, "en-us", skill_id="test.skill")
+
+    def test_vocabularies_map(self):
+        resources = self._resources({
+            "thing.voc": "widget\ngadget\n# comment\n",
+            "foo.intent": "what about <thing>\n",
+        })
+        self.assertEqual(resources.vocabularies().get("thing"),
+                         ["widget", "gadget"])
+
+    def test_vocabularies_expands_alternates(self):
+        resources = self._resources({
+            "greeting.voc": "(hello|hi)\ngood morning\n",
+        })
+        self.assertEqual(sorted(resources.vocabularies()["greeting"]),
+                         ["good morning", "hello", "hi"])
+
+
+class TestInlineVocabRegistration(unittest.TestCase):
+    """The samples emitted on the registration bus topic must already have the
+    inline ``<name>`` resolved to an ``(a|b|c)`` alternation group."""
+
+    def _capture_samples(self, files, pass_vocabs):
+        tmp = TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        locale = Path(tmp.name, "locale", "en-us")
+        locale.mkdir(parents=True)
+        for name, content in files.items():
+            (locale / name).write_text(content)
+        resources = SkillResources(tmp.name, "en-us", skill_id="test.skill")
+        filename = str(ResourceFile(resources.types.intent, "foo.intent").file_path)
+
+        bus = FakeBus()
+        captured = {}
+        bus.on("padatious:register_intent",
+               lambda m: captured.__setitem__("samples", m.data["samples"]))
+        iface = IntentServiceInterface(bus)
+        iface.set_id("test.skill")
+        kwargs = {"vocabs": resources.vocabularies()} if pass_vocabs else {}
+        iface.register_padatious_intent("test.skill:foo.intent", filename,
+                                        "en-us", **kwargs)
+        return captured["samples"]
+
+    def test_inline_reference_resolved_before_engine(self):
+        samples = self._capture_samples({
+            "thing.voc": "widget\ngadget\n",
+            "foo.intent": "what about <thing>\n",
+        }, pass_vocabs=True)
+        self.assertEqual(samples, ["what about (widget|gadget)"])
+
+    def test_raw_reference_without_vocabs_is_unresolved(self):
+        samples = self._capture_samples({
+            "thing.voc": "widget\ngadget\n",
+            "foo.intent": "what about <thing>\n",
+        }, pass_vocabs=False)
+        self.assertEqual(samples, ["what about <thing>"])
+
+
+class TestInlineVocabEngineEndToEnd(unittest.TestCase):
+    """End-to-end proof through the real engines: a skill shipping a ``foo.intent``
+    with ``<thing>`` and a sibling ``thing.voc`` matches an utterance built from a
+    vocabulary member, but only when the reference is resolved at registration."""
+
+    def _samples(self, pass_vocabs):
+        tmp = TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        locale = Path(tmp.name, "locale", "en-us")
+        locale.mkdir(parents=True)
+        (locale / "thing.voc").write_text("widget\ngadget\n")
+        (locale / "foo.intent").write_text("what about <thing>\n")
+        resources = SkillResources(tmp.name, "en-us", skill_id="test.skill")
+        filename = str(ResourceFile(resources.types.intent, "foo.intent").file_path)
+
+        bus = FakeBus()
+        captured = {}
+        bus.on("padatious:register_intent",
+               lambda m: captured.__setitem__("samples", m.data["samples"]))
+        iface = IntentServiceInterface(bus)
+        iface.set_id("test.skill")
+        kwargs = {"vocabs": resources.vocabularies()} if pass_vocabs else {}
+        iface.register_padatious_intent("test.skill:foo.intent", filename,
+                                        "en-us", **kwargs)
+        return captured["samples"]
+
+    def test_padacioso_inline_voc_intent_matches(self):
+        from padacioso import IntentContainer
+        engine = IntentContainer()
+        engine.add_intent("foo", self._samples(pass_vocabs=True))
+        self.assertEqual(engine.calc_intent("what about widget").get("name"),
+                         "foo")
+
+    def test_padacioso_raw_reference_is_rejected(self):
+        # an unresolved <thing> never reaches the engine as a matchable sample:
+        # padacioso cannot build an intent from the dangling reference
+        from padacioso import IntentContainer
+        from ovos_spec_tools.expansion import MalformedTemplate
+        engine = IntentContainer()
+        with self.assertRaises(MalformedTemplate):
+            engine.add_intent("foo", self._samples(pass_vocabs=False))
+
+
 if __name__ == "__main__":
     unittest.main()