diff --git a/ovos_workshop/intents.py b/ovos_workshop/intents.py index 729eeedd..c6c237c9 100644 --- a/ovos_workshop/intents.py +++ b/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 `.blacklist`). + @param vocabs: ``{name: members}`` of the sibling vocabularies, so an + inline ```` 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 in place + samples = [inline_keywords(s, vocabs) for s in samples] data = {'file_name': filename, "samples": samples, 'name': intent_name, diff --git a/ovos_workshop/resource_files.py b/ovos_workshop/resource_files.py index ef58d7cf..03f19806 100644 --- a/ovos_workshop/resource_files.py +++ b/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 ```` 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 diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 1236c636..90df5dd1 100644 --- a/ovos_workshop/skills/ovos.py +++ b/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 + # 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) diff --git a/test/unittests/skills/test_base.py b/test/unittests/skills/test_base.py index 07e48797..b8088928 100644 --- a/test/unittests/skills/test_base.py +++ b/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) diff --git a/test/unittests/test_inline_vocab_refs.py b/test/unittests/test_inline_vocab_refs.py index 3117602b..1d71115f 100644 --- a/test/unittests/test_inline_vocab_refs.py +++ b/test/unittests/test_inline_vocab_refs.py @@ -1,12 +1,22 @@ +"""OVOS-INTENT-1 §3.7: an inline ```` 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 ```` +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 ```` 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 ```` 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 \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 ```` 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 \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 \n", + }, pass_vocabs=False) + self.assertEqual(samples, ["what about "]) + + +class TestInlineVocabEngineEndToEnd(unittest.TestCase): + """End-to-end proof through the real engines: a skill shipping a ``foo.intent`` + with ```` 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 \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 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()