Skip to content

Commit d2d34c0

Browse files
JarbasAlclaude
andcommitted
fix: expand inline <voc> refs before registering intents to the engine
OVOS-INTENT-1 §3.7 makes an inline <name> in a .intent an authoring convenience that MUST expand in place from the sibling vocabulary of that name before the template reaches an intent engine. The padatious/padacioso bus protocol carries only samples, never the .voc content, so register_intent_file sent the raw <name> straight through and both engines trained on the literal token — padacioso never matched and padatious matched only by coincidence. Add SkillResources.vocabularies() to build the {name: members} map from the locale tree (user > skill > workshop precedence) and pass it into register_padatious_intent, which now inlines every <name> as an (a|b|c) group via ovos_spec_tools.inline_keywords before emitting the samples. Files without inline references are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 72993f3 commit d2d34c0

5 files changed

Lines changed: 161 additions & 15 deletions

File tree

ovos_workshop/intents.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# their long-standing `from ovos_workshop.intents import IntentBuilder` import while
1212
# the single source of truth is the spec. The `IntentServiceInterface` producer
1313
# (and the munge_* helpers) below consume these definitions.
14-
from ovos_spec_tools import Intent, IntentBuilder, open_intent_envelope
14+
from ovos_spec_tools import Intent, IntentBuilder, inline_keywords, open_intent_envelope
1515

1616

1717
def to_alnum(skill_id: str) -> str:
@@ -271,7 +271,8 @@ def remove_adapt_context(self, context: str):
271271

272272
def register_padatious_intent(self, intent_name: str, filename: str,
273273
lang: str, string_blacklist: Optional[List[str]] = None,
274-
slot_blacklist: Optional[Dict[str, List[str]]] = None):
274+
slot_blacklist: Optional[Dict[str, List[str]]] = None,
275+
vocabs: Optional[Dict[str, List[str]]] = None):
275276
"""
276277
Register a Padatious intent file with the intent service.
277278
@param intent_name: Unique intent identifier
@@ -283,6 +284,11 @@ def register_padatious_intent(self, intent_name: str, filename: str,
283284
@param slot_blacklist: OVOS-INTENT-2 §4.3 slot-value exclusions keyed by
284285
slot name, ``{slot: [excluded phrases]}`` — values that MUST NOT
285286
fill the named ``{slot}`` (each slot's sibling `<slot>.blacklist`).
287+
@param vocabs: ``{name: members}`` of the sibling vocabularies, so an
288+
inline ``<name>`` reference (OVOS-INTENT-1 §3.7) is baked into the
289+
samples as an ``(a|b|c)`` group before they reach the engine — the
290+
padatious/padacioso bus protocol carries only samples, never the
291+
``.voc`` content, so the reference must be resolved here.
286292
"""
287293
if not isinstance(filename, str):
288294
raise ValueError('Filename path must be a string')
@@ -291,6 +297,9 @@ def register_padatious_intent(self, intent_name: str, filename: str,
291297
with open(filename) as f:
292298
samples = [_ for _ in f.read().split("\n") if _
293299
and not _.startswith("#")]
300+
if vocabs:
301+
# OVOS-INTENT-1 §3.7: resolve every inline <name> in place
302+
samples = [inline_keywords(s, vocabs) for s in samples]
294303
data = {'file_name': filename,
295304
"samples": samples,
296305
'name': intent_name,

ovos_workshop/resource_files.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -728,11 +728,13 @@ def _locale_vocabularies(self) -> Dict[str, List[str]]:
728728
name = file_name[:-len(".voc")].lower()
729729
if name in vocabularies:
730730
continue
731-
members = []
731+
members: List[str] = []
732732
with open(Path(directory, file_name)) as voc_file:
733-
for line in (ln.strip() for ln in voc_file):
733+
for line in (ln.strip().lower() for ln in voc_file):
734734
if line and not line.startswith("#"):
735-
members.append(line.lower())
735+
# a .voc line MAY hold comma/parenthesis
736+
# alternates; every phrasing is a member
737+
members.extend(flatten_list(expand_template(line)))
736738
if members:
737739
vocabularies[name] = members
738740
return vocabularies
@@ -896,6 +898,20 @@ def load_skill_vocabulary(self, alphanumeric_skill_id: str) -> dict:
896898

897899
return skill_vocabulary
898900

901+
def vocabularies(self) -> Dict[str, List[str]]:
902+
"""Build a ``{name: members}`` map from every ``.voc`` in this
903+
language's locale tree.
904+
905+
Feeds the inline ``<name>`` resolution required by OVOS-INTENT-1 §3.7:
906+
a reference in an ``.intent`` (or ``.blacklist``) expands in place from
907+
the sibling vocabulary of that name. Padatious and padacioso never read
908+
``.voc`` files at match time, so the reference must be baked into the
909+
template before it reaches the engine. User overrides take precedence
910+
over the skill's own resources, which take precedence over
911+
workshop-shipped resources.
912+
"""
913+
return self._locale_vocabularies()
914+
899915
def load_skill_regex(self, alphanumeric_skill_id: str) -> List[str]:
900916
"""
901917
Load all regex files in the skill's resources

ovos_workshop/skills/ovos.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1365,9 +1365,12 @@ def register_intent_file(self, intent_file: str, handler: callable,
13651365
if phrases:
13661366
slot_blacklist[slot] = phrases
13671367

1368+
# OVOS-INTENT-1 §3.7: supply the sibling vocabularies so an inline
1369+
# <name> reference in the .intent is baked into the samples before
1370+
# they are sent to the engine over the bus
13681371
self.intent_service.register_padatious_intent(
13691372
name, filename, lang, string_blacklist=disallowed_strings,
1370-
slot_blacklist=slot_blacklist)
1373+
slot_blacklist=slot_blacklist, vocabs=resources.vocabularies())
13711374
if handler:
13721375
self.add_event(name, handler, 'mycroft.skill.handler',
13731376
activation=True, is_intent=True)

test/unittests/skills/test_base.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from logging import Logger
2020
from threading import Event, Thread
2121
from time import time
22-
from unittest.mock import Mock
22+
from unittest.mock import ANY, Mock
2323
from os.path import join, dirname, isdir
2424
from ovos_workshop.skills.ovos import OVOSSkill
2525

@@ -443,7 +443,7 @@ def test_register_intent_file(self):
443443
skill.config_core["secondary_langs"] = []
444444
skill.register_intent_file("time.intent", Mock(__name__="test"))
445445
skill.intent_service.register_padatious_intent.assert_called_once_with(
446-
f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={})
446+
f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={}, vocabs=ANY)
447447

448448
# With secondary language
449449
skill.intent_service.register_padatious_intent.reset_mock()
@@ -452,9 +452,9 @@ def test_register_intent_file(self):
452452
self.assertEqual(
453453
skill.intent_service.register_padatious_intent.call_count, 2)
454454
skill.intent_service.register_padatious_intent.assert_any_call(
455-
f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={})
455+
f"{skill.skill_id}:time.intent", en_intent_file, "en-US", string_blacklist=[], slot_blacklist={}, vocabs=ANY)
456456
skill.intent_service.register_padatious_intent.assert_any_call(
457-
f"{skill.skill_id}:time.intent", uk_intent_file, "uk-UA", string_blacklist=[], slot_blacklist={})
457+
f"{skill.skill_id}:time.intent", uk_intent_file, "uk-UA", string_blacklist=[], slot_blacklist={}, vocabs=ANY)
458458

459459
def test_register_entity_file(self):
460460
skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)

test/unittests/test_inline_vocab_refs.py

Lines changed: 123 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,22 @@
1+
"""OVOS-INTENT-1 §3.7: an inline ``<name>`` reference in a ``.intent`` file is
2+
an authoring convenience that must expand in place from the sibling ``.voc`` of
3+
that name before the template reaches an intent engine. The padatious/padacioso
4+
bus protocol carries only samples — never the ``.voc`` content — so the
5+
reference has to be resolved during registration, otherwise the raw ``<name>``
6+
token is trained into the engine and the intent never matches."""
17
import unittest
2-
from tempfile import TemporaryDirectory
38
from pathlib import Path
9+
from tempfile import TemporaryDirectory
10+
11+
from ovos_utils.fakebus import FakeBus
12+
from ovos_workshop.intents import IntentServiceInterface
13+
from ovos_workshop.resource_files import ResourceFile, SkillResources
414

515

616
class TestInlineVocabReferences(unittest.TestCase):
7-
"""OVOS-INTENT-1 §3.7: an inline ``<name>`` reference in a ``.intent`` or
8-
``.blacklist`` file must expand in place from the sibling ``.voc`` of that
9-
name found in the same locale directory."""
17+
"""The load-time resolution path: ``load_intent_file`` / ``load_blacklist_file``
18+
expand every inline ``<name>`` from the sibling ``.voc`` of that name found in
19+
the same locale directory."""
1020

1121
def _skill_dir(self, files: dict) -> str:
1222
tmp = TemporaryDirectory()
@@ -18,7 +28,6 @@ def _skill_dir(self, files: dict) -> str:
1828
return tmp.name
1929

2030
def _resources(self, skill_dir):
21-
from ovos_workshop.resource_files import SkillResources
2231
return SkillResources(skill_dir, "en-us", skill_id="test.skill")
2332

2433
def test_intent_inline_vocab(self):
@@ -48,5 +57,114 @@ def test_intent_without_inline_ref_unchanged(self):
4857
self.assertIn("turn on the light", intents)
4958

5059

60+
class TestInlineVocabResources(unittest.TestCase):
61+
def _resources(self, files):
62+
tmp = TemporaryDirectory()
63+
self.addCleanup(tmp.cleanup)
64+
locale = Path(tmp.name, "locale", "en-us")
65+
locale.mkdir(parents=True)
66+
for name, content in files.items():
67+
(locale / name).write_text(content)
68+
return SkillResources(tmp.name, "en-us", skill_id="test.skill")
69+
70+
def test_vocabularies_map(self):
71+
resources = self._resources({
72+
"thing.voc": "widget\ngadget\n# comment\n",
73+
"foo.intent": "what about <thing>\n",
74+
})
75+
self.assertEqual(resources.vocabularies().get("thing"),
76+
["widget", "gadget"])
77+
78+
def test_vocabularies_expands_alternates(self):
79+
resources = self._resources({
80+
"greeting.voc": "(hello|hi)\ngood morning\n",
81+
})
82+
self.assertEqual(sorted(resources.vocabularies()["greeting"]),
83+
["good morning", "hello", "hi"])
84+
85+
86+
class TestInlineVocabRegistration(unittest.TestCase):
87+
"""The samples emitted on the registration bus topic must already have the
88+
inline ``<name>`` resolved to an ``(a|b|c)`` alternation group."""
89+
90+
def _capture_samples(self, files, pass_vocabs):
91+
tmp = TemporaryDirectory()
92+
self.addCleanup(tmp.cleanup)
93+
locale = Path(tmp.name, "locale", "en-us")
94+
locale.mkdir(parents=True)
95+
for name, content in files.items():
96+
(locale / name).write_text(content)
97+
resources = SkillResources(tmp.name, "en-us", skill_id="test.skill")
98+
filename = str(ResourceFile(resources.types.intent, "foo.intent").file_path)
99+
100+
bus = FakeBus()
101+
captured = {}
102+
bus.on("padatious:register_intent",
103+
lambda m: captured.__setitem__("samples", m.data["samples"]))
104+
iface = IntentServiceInterface(bus)
105+
iface.set_id("test.skill")
106+
kwargs = {"vocabs": resources.vocabularies()} if pass_vocabs else {}
107+
iface.register_padatious_intent("test.skill:foo.intent", filename,
108+
"en-us", **kwargs)
109+
return captured["samples"]
110+
111+
def test_inline_reference_resolved_before_engine(self):
112+
samples = self._capture_samples({
113+
"thing.voc": "widget\ngadget\n",
114+
"foo.intent": "what about <thing>\n",
115+
}, pass_vocabs=True)
116+
self.assertEqual(samples, ["what about (widget|gadget)"])
117+
118+
def test_raw_reference_without_vocabs_is_unresolved(self):
119+
samples = self._capture_samples({
120+
"thing.voc": "widget\ngadget\n",
121+
"foo.intent": "what about <thing>\n",
122+
}, pass_vocabs=False)
123+
self.assertEqual(samples, ["what about <thing>"])
124+
125+
126+
class TestInlineVocabEngineEndToEnd(unittest.TestCase):
127+
"""End-to-end proof through the real engines: a skill shipping a ``foo.intent``
128+
with ``<thing>`` and a sibling ``thing.voc`` matches an utterance built from a
129+
vocabulary member, but only when the reference is resolved at registration."""
130+
131+
def _samples(self, pass_vocabs):
132+
tmp = TemporaryDirectory()
133+
self.addCleanup(tmp.cleanup)
134+
locale = Path(tmp.name, "locale", "en-us")
135+
locale.mkdir(parents=True)
136+
(locale / "thing.voc").write_text("widget\ngadget\n")
137+
(locale / "foo.intent").write_text("what about <thing>\n")
138+
resources = SkillResources(tmp.name, "en-us", skill_id="test.skill")
139+
filename = str(ResourceFile(resources.types.intent, "foo.intent").file_path)
140+
141+
bus = FakeBus()
142+
captured = {}
143+
bus.on("padatious:register_intent",
144+
lambda m: captured.__setitem__("samples", m.data["samples"]))
145+
iface = IntentServiceInterface(bus)
146+
iface.set_id("test.skill")
147+
kwargs = {"vocabs": resources.vocabularies()} if pass_vocabs else {}
148+
iface.register_padatious_intent("test.skill:foo.intent", filename,
149+
"en-us", **kwargs)
150+
return captured["samples"]
151+
152+
def test_padacioso_inline_voc_intent_matches(self):
153+
from padacioso import IntentContainer
154+
engine = IntentContainer()
155+
engine.add_intent("foo", self._samples(pass_vocabs=True))
156+
self.assertEqual(engine.calc_intent("what about widget").get("name"),
157+
"foo")
158+
159+
def test_padacioso_raw_reference_is_rejected(self):
160+
# an unresolved <thing> never reaches the engine as a matchable sample:
161+
# padacioso cannot build an intent from the dangling reference
162+
from padacioso import IntentContainer
163+
from ovos_spec_tools.expansion import MalformedTemplate
164+
engine = IntentContainer()
165+
with self.assertRaises(MalformedTemplate):
166+
engine.add_intent("foo", self._samples(pass_vocabs=False))
167+
168+
51169
if __name__ == "__main__":
52170
unittest.main()

0 commit comments

Comments
 (0)