diff --git a/ovos_workshop/intents.py b/ovos_workshop/intents.py index 6453e5de..2c5a5b23 100644 --- a/ovos_workshop/intents.py +++ b/ovos_workshop/intents.py @@ -145,17 +145,21 @@ def munge_intent_parser(intent_parser, name, skill_id): # dual-emit (see IntentServiceInterface.register_keyword/register_intent) # ------------------------------------------------------------------ - def emit_legacy_register_vocab(self, vocab_type: str, entity: str, + def emit_legacy_register_vocab(self, msg: Message, vocab_type: str, + entity: str, aliases: Optional[List[str]] = None, lang: str = None): """Emit the legacy adapt ``register_vocab`` topic (entity + aliases). + `msg` is the caller's stamped copy, carrying this skill as the + producer. The adapt engine reads the legacy producer from the + context, so digging the ambient message here would attribute the + vocabulary to whichever component the skill happens to be handling + while the intent it belongs to went out attributed correctly. + TODO: drop once the adapt pipeline consumes ovos.intent.register.keyword (INTENT-4 ยง5) directly. """ aliases = aliases or [] - msg = dig_for_message() or Message("") - if "skill_id" not in msg.context: - msg.context["skill_id"] = self.skill_id entity_data = {'entity_value': entity, 'entity_type': vocab_type, 'lang': lang} @@ -480,7 +484,8 @@ def register_keyword(self, vocab_type: str, entity: str, samples.append(value) # TODO: drop once _AdaptIntentApi.emit_legacy_register_vocab is removed. - self._adapt.emit_legacy_register_vocab(vocab_type, entity, aliases, lang) + self._adapt.emit_legacy_register_vocab(msg, vocab_type, entity, + aliases, lang) def _unmunge_vocab_name(self, vocab_type: str) -> str: prefix = _AdaptIntentApi.to_alnum(self.skill_id) diff --git a/test/unittests/test_intent4_producer.py b/test/unittests/test_intent4_producer.py index 281df772..6bde9736 100644 --- a/test/unittests/test_intent4_producer.py +++ b/test/unittests/test_intent4_producer.py @@ -471,6 +471,53 @@ def test_munge_and_register_intent_still_spec_emits_keyword(self): self.assertEqual(data["required"], [{"name": "kw", "samples": ["hello"]}]) +class LegacyVocabProducerIdentityTest(unittest.TestCase): + """The legacy register_vocab names the registering skill as its producer. + + The adapt engine reads the legacy producer from the context, so a skill + that registers while handling another component's message must not ship + that component as the owner of its vocabulary. + """ + + def setUp(self): + self.bus = CapturingBus() + self.iface = IntentServiceInterface(self.bus) + self.iface.set_id("test.skill") + self.foreign = Message("some.other.event", {}, + {"skill_id": "other.component"}) + + def test_legacy_vocab_is_attributed_to_the_registering_skill(self): + with patch("ovos_workshop.intents.dig_for_message", + return_value=self.foreign): + self.iface.register_adapt_keyword("setKW", "set", + aliases=["change"], + lang="en-US") + legacy = self.bus.of_type("register_vocab") + self.assertEqual(len(legacy), 2) # entity + one alias + for _, context in legacy: + self.assertEqual(context["skill_id"], "test.skill") + + def test_legacy_and_spec_agree_on_the_producer(self): + # The vocabulary and the intent that references it are one + # registration; disagreeing between them splits it across two owners. + with patch("ovos_workshop.intents.dig_for_message", + return_value=self.foreign): + self.iface.register_adapt_keyword("setKW", "set", lang="en-US") + parser = IntentBuilder("set_it").require("setKW").build() + self.iface.register_adapt_intent("set_it", parser) + spec = self.bus.of_type(SpecMessage.INTENT_REGISTER_KEYWORD) + legacy = self.bus.of_type("register_vocab") + self.assertTrue(spec and legacy) + owners = {c["skill_id"] for _, c in spec + legacy} + self.assertEqual(owners, {"test.skill"}) + + def test_the_dug_message_is_not_mutated(self): + with patch("ovos_workshop.intents.dig_for_message", + return_value=self.foreign): + self.iface.register_adapt_keyword("setKW", "set", lang="en-US") + self.assertEqual(self.foreign.context["skill_id"], "other.component") + + class RegexRegistrationTest(unittest.TestCase): """Regex intents are adapt-engine only; the surviving registration name is register_adapt_regex."""