From 9280dc826bd72fc155d495ea7df03257dfae5a0e Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 18 Mar 2026 22:37:39 +0000 Subject: [PATCH 01/22] feat: yesno/selection agent plugins --- docs/skill-interaction.md | 357 +++++++++++++++++++++++ ovos_workshop/skills/ovos.py | 63 +++- pyproject.toml | 1 + test/unittests/test_skill_interaction.py | 264 +++++++++++++++++ 4 files changed, 674 insertions(+), 11 deletions(-) create mode 100644 docs/skill-interaction.md create mode 100644 test/unittests/test_skill_interaction.py diff --git a/docs/skill-interaction.md b/docs/skill-interaction.md new file mode 100644 index 00000000..c15413ea --- /dev/null +++ b/docs/skill-interaction.md @@ -0,0 +1,357 @@ +# Skill Interaction Methods + +`OVOSSkill` provides two high-level methods for collecting structured user input: `ask_yesno` for binary yes/no questions and `ask_selection` for multiple-choice prompts. Both are backed by pluggable agent engines that can be swapped per-skill or system-wide. + +--- + +## ask_yesno + +```python +OVOSSkill.ask_yesno(prompt: str, data: Optional[dict] = None) -> Optional[str] +``` + +Speaks *prompt*, waits for the user's response, and classifies it as `"yes"`, `"no"`, or the raw response string if neither matched. + +**Source**: `OVOSSkill.ask_yesno` — `ovos_workshop/skills/ovos.py` + +### Parameters + +| Parameter | Type | Description | +|-----------|------|-------------| +| `prompt` | `str` | Dialog ID (looked up in `locale/`) or a literal string to speak. | +| `data` | `dict \| None` | Template variables for Mustache rendering of the dialog string. | + +### Return values + +| Value | Meaning | +|-------|---------| +| `"yes"` | User confirmed (e.g. "yeah", "sure", "of course") | +| `"no"` | User declined (e.g. "nope", "nah", "definitely not") | +| `str` | User spoke something that could not be classified — raw transcript returned | +| `None` | No response received (timeout or user said nothing) | + +### Basic usage + +```python +class MySkill(OVOSSkill): + def handle_delete_intent(self, message): + if self.ask_yesno("confirm_delete") == "yes": + self._do_delete() + self.speak_dialog("deleted") + else: + self.speak_dialog("cancelled") +``` + +`locale/en-us/confirm_delete.dialog`: +``` +Are you sure you want to delete this? +Do you really want to delete it? +``` + +### With template data + +```python +answer = self.ask_yesno("confirm_action", data={"action": "restart the server"}) +``` + +`locale/en-us/confirm_action.dialog`: +``` +Are you sure you want to {{action}}? +``` + +### Handling all return values + +```python +answer = self.ask_yesno("do_you_want_music") +if answer == "yes": + self.play_music() +elif answer == "no": + self.speak_dialog("okay_nevermind") +elif answer is None: + self.speak_dialog("no_response") +else: + # answer is the raw transcript — user said something unexpected + self.speak_dialog("did_not_understand") +``` + +### How it works internally + +1. `get_response(dialog=prompt, data=data)` — speaks the prompt, records user reply. +2. `_get_yesno_engine()` — loads the configured `YesNoEngine` plugin (if any). +3. If a plugin is loaded: `engine.yes_or_no(question=prompt, response=resp, lang=self.lang)` → `True`, `False`, or `None`. +4. If no plugin: `YesNoSolver().match_yes_or_no(resp, lang=self.lang)` (built-in fallback, always available). +5. `True` → `"yes"`, `False` → `"no"`, `None`/unmatched → raw response. + +--- + +## ask_selection + +```python +OVOSSkill.ask_selection( + options: List[str], + dialog: str = '', + data: Optional[dict] = None, + min_conf: float = 0.65, + numeric: bool = False, + num_retries: int = -1, +) -> Optional[str] +``` + +Speaks the options list to the user, optionally follows with a dialog prompt, then resolves the user's spoken response to one of the options. + +**Source**: `OVOSSkill.ask_selection` — `ovos_workshop/skills/ovos.py` + +### Parameters + +| Parameter | Type | Default | Description | +|-----------|------|---------|-------------| +| `options` | `List[str]` | — | The predefined options to offer. | +| `dialog` | `str` | `''` | Dialog ID or literal string spoken **after** the options list. | +| `data` | `dict \| None` | `None` | Template variables for the dialog string. | +| `min_conf` | `float` | `0.65` | Minimum fuzzy-match confidence for the default plugin. Passed to the `OptionMatcherEngine` config if no plugin-level config overrides it. | +| `numeric` | `bool` | `False` | If `True`, speaks each option prefixed with its number ("one, pizza; two, pasta; …"). If `False`, speaks them as a joined list ("pizza, pasta, or salad?"). | +| `num_retries` | `int` | `-1` | How many times to re-prompt on no response. `-1` means use the system default. | + +### Return values + +| Value | Meaning | +|-------|---------| +| `str` | One of the strings from `options`, exactly as provided. | +| `None` | No match, no response, or plugin failure. | + +Special cases handled before user interaction: +- Empty `options` → `None` immediately. +- Single-element `options` → returns that element immediately (no prompt). + +### Basic usage + +```python +class MySkill(OVOSSkill): + def handle_transport_intent(self, message): + modes = ["bus", "train", "bicycle"] + choice = self.ask_selection(modes, dialog="which_transport") + if choice: + self.speak_dialog("you_chose", {"mode": choice}) +``` + +`locale/en-us/which_transport.dialog`: +``` +Which would you prefer? +How would you like to travel? +``` + +The skill speaks: *"bus, train, or bicycle? Which would you prefer?"* + +The user can say: +- `"train"` — fuzzy-matched directly +- `"the second one"` / `"number two"` / `"two"` — position matched +- `"the last one"` — last-word matched +- `"option 3"` — numeric matched (requires `ovos-number-parser`) + +### Numeric menu + +```python +choice = self.ask_selection(options, numeric=True) +``` + +Speaks each option as: *"one, bus; two, train; three, bicycle"*. Useful when options are long or ambiguous. The user can then say *"two"* or *"the second one"*. + +### Handling None + +```python +choice = self.ask_selection(options, dialog="which_one", num_retries=1) +if choice is None: + self.speak_dialog("could_not_understand") + return +``` + +### How it works internally + +1. Validates `options` (raises `ValueError` if not a list; returns immediately for 0 or 1 items). +2. Speaks options (as list or numbered menu based on `numeric`). +3. `get_response(dialog=dialog, data=data, num_retries=num_retries)` — speaks optional follow-up dialog, records reply. +4. `_get_selection_engine()` — loads the configured `OptionMatcherEngine` plugin. +5. `engine.match_option(utterance=resp, options=options, lang=self.lang)` — resolves response to a slot. +6. If the engine raises or returns `None`, `ask_selection` returns `None`. + +--- + +## Plugin system + +Both methods are backed by pluggable agent engines discovered and loaded via [ovos-plugin-manager](https://github.com/OpenVoiceOS/ovos-plugin-manager). + +### ask_yesno — YesNoEngine + +**Plugin type**: `YesNoEngine` (`opm.agents.yesno`) +**Config key**: `ask_yesno_plugin` +**Built-in fallback**: `ovos-solver-yes-no-plugin` (always available, no config needed) + +`YesNoEngine` plugins implement: + +```python +def yes_or_no(self, question: str, response: str, lang: Optional[str] = None) -> Optional[bool]: + ... # True = yes, False = no, None = unclear +``` + +The `question` argument gives the plugin context about what was asked, enabling smarter inference (e.g. an LLM-backed plugin could use it to resolve ambiguous answers). + +**Available plugins**: + +| Plugin | Description | +|--------|-------------| +| `ovos-solver-yes-no-plugin` | Rule-based multilingual yes/no classifier (default fallback) | + +### ask_selection — OptionMatcherEngine + +**Plugin type**: `OptionMatcherEngine` (`opm.agents.option_matcher`) +**Config key**: `ask_selection_plugin` +**Default plugin**: `ovos-option-matcher-fuzzy-plugin` (installed as a dependency of `ovos-workshop`) + +`OptionMatcherEngine` plugins implement: + +```python +def match_option(self, utterance: str, options: List[str], lang: Optional[str] = None) -> Optional[str]: + ... # returns one of options, or None +``` + +**Available plugins**: + +| Plugin | Description | +|--------|-------------| +| `ovos-option-matcher-fuzzy-plugin` | Fuzzy + ordinal/cardinal vocab + numeric fallback. Default. | + +--- + +## Configuration + +### Global defaults — `mycroft.conf` + +```json +{ + "skills": { + "ask_yesno_plugin": "ovos-solver-yes-no-plugin", + "ask_selection_plugin": "ovos-option-matcher-fuzzy-plugin" + } +} +``` + +`ask_yesno_plugin` defaults to `None` (built-in `YesNoSolver` used). `ask_selection_plugin` defaults to `ovos-option-matcher-fuzzy-plugin`. + +### Per-skill override — `settings.json` + +Place in the skill's `settings.json` to override for that skill only: + +```json +{ + "ask_yesno_plugin": "my-llm-yesno-plugin", + "ask_selection_plugin": "my-embedding-option-matcher" +} +``` + +### Plugin config + +Pass configuration to the plugin via the same settings block: + +```json +{ + "ask_selection_plugin": "ovos-option-matcher-fuzzy-plugin", + "ask_selection_plugin_config": { + "min_conf": 0.80 + } +} +``` + +### Priority order + +``` +settings.json > mycroft.conf skills block > built-in default +``` + +Plugins are loaded lazily on first use and cached per plugin name for the lifetime of the skill instance. + +--- + +## Writing a custom YesNoEngine plugin + +```python +# my_yesno/__init__.py +from typing import Optional +from ovos_plugin_manager.templates.agents import YesNoEngine + +class MyYesNoPlugin(YesNoEngine): + def yes_or_no(self, question: str, response: str, + lang: Optional[str] = None) -> Optional[bool]: + r = response.lower() + if "yes" in r or "sure" in r: + return True + if "no" in r or "never" in r: + return False + return None # unclear +``` + +```toml +# pyproject.toml +[project.entry-points."opm.agents.yesno"] +my-yesno-plugin = "my_yesno:MyYesNoPlugin" +``` + +Activate for a skill: + +```json +{ "ask_yesno_plugin": "my-yesno-plugin" } +``` + +--- + +## Writing a custom OptionMatcherEngine plugin + +```python +# my_matcher/__init__.py +from typing import List, Optional +from ovos_plugin_manager.templates.agents import OptionMatcherEngine + +class MyOptionMatcher(OptionMatcherEngine): + def match_option(self, utterance: str, options: List[str], + lang: Optional[str] = None) -> Optional[str]: + # example: embedding similarity via a local model + scores = self._embed_and_score(utterance, options) + best_idx = max(range(len(scores)), key=lambda i: scores[i]) + if scores[best_idx] >= self.config.get("min_conf", 0.6): + return options[best_idx] + return None +``` + +```toml +# pyproject.toml +[project.entry-points."opm.agents.option_matcher"] +my-option-matcher-plugin = "my_matcher:MyOptionMatcher" +``` + +Activate system-wide: + +```json +{ "skills": { "ask_selection_plugin": "my-option-matcher-plugin" } } +``` + +--- + +## Failure behaviour + +| Scenario | ask_yesno result | ask_selection result | +|----------|-----------------|---------------------| +| User says nothing (timeout) | `None` | `None` | +| User response unclassifiable | raw transcript string | `None` | +| Plugin fails to load | falls back to `YesNoSolver` | `None` | +| Plugin raises at runtime | falls back to `YesNoSolver` | `None` | +| No plugin configured | `YesNoSolver` used | default fuzzy plugin used | + +`ask_selection` is intentionally strict: any failure returns `None` rather than guessing. Always handle the `None` case in your skill. + +--- + +## See also + +- [`ovos-solver-yes-no-plugin`](https://github.com/OpenVoiceOS/ovos-solver-YesNo-plugin) — built-in yes/no classifier +- [`ovos-option-matcher-fuzzy-plugin`](https://github.com/OpenVoiceOS/ovos-option-matcher-fuzzy-plugin) — default selection plugin, with [full docs](https://github.com/OpenVoiceOS/ovos-option-matcher-fuzzy-plugin/tree/master/docs) +- `OVOSSkill.get_response` — lower-level method used internally by both +- [OPM agent templates](https://github.com/OpenVoiceOS/ovos-plugin-manager/blob/dev/ovos_plugin_manager/templates/agents.py) — `YesNoEngine`, `OptionMatcherEngine` base classes diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index cf5c69c3..013ff6f8 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -1928,6 +1928,47 @@ def acknowledge(self): 'snd/acknowledge.mp3') self.play_audio(audio_file, instant=True) + def _get_yesno_engine(self) -> Optional[object]: + """Load the configured YesNoEngine plugin, with per-skill override support. + + Checks settings.json first, then mycroft.conf skills.ask_yesno_plugin. + Returns None if no plugin is configured, preserving built-in fallback behavior. + """ + plugin_name = (self.settings.get("ask_yesno_plugin") or + self.config_core.get("skills", {}).get("ask_yesno_plugin")) + if not plugin_name: + return None + cache_key = f"__yesno_engine_{plugin_name}" + if not hasattr(self, cache_key): + try: + from ovos_plugin_manager.agents import load_yesno_plugin + cls = load_yesno_plugin(plugin_name) + setattr(self, cache_key, cls()) + except Exception as e: + LOG.error(f"Failed to load YesNo plugin '{plugin_name}': {e}") + setattr(self, cache_key, None) + return getattr(self, cache_key) + + def _get_selection_engine(self) -> Optional[object]: + """Load the configured OptionMatcherEngine plugin, with per-skill override support. + + Checks settings.json first, then mycroft.conf skills.ask_selection_plugin, + defaulting to ovos-option-matcher-fuzzy-plugin when neither is set. + """ + plugin_name = (self.settings.get("ask_selection_plugin") or + self.config_core.get("skills", {}).get("ask_selection_plugin") or + "ovos-option-matcher-fuzzy-plugin") + cache_key = f"__selection_engine_{plugin_name}" + if not hasattr(self, cache_key): + try: + from ovos_plugin_manager.agents import load_option_matcher_plugin + cls = load_option_matcher_plugin(plugin_name) + setattr(self, cache_key, cls()) + except Exception as e: + LOG.error(f"Failed to load selection plugin '{plugin_name}': {e}") + setattr(self, cache_key, None) + return getattr(self, cache_key) + def ask_yesno(self, prompt: str, data: Optional[dict] = None) -> Optional[str]: """ @@ -1939,7 +1980,11 @@ def ask_yesno(self, prompt: str, 'no', including a response of None. """ resp = self.get_response(dialog=prompt, data=data) - answer = YesNoSolver().match_yes_or_no(resp, lang=self.lang) if resp else resp + engine = self._get_yesno_engine() + if engine is not None: + answer = engine.yes_or_no(question=prompt, response=resp, lang=self.lang) if resp else None + else: + answer = YesNoSolver().match_yes_or_no(resp, lang=self.lang) if resp else resp if answer is True: return "yes" elif answer is False: @@ -1990,17 +2035,13 @@ def ask_selection(self, options: List[str], dialog: str = '', resp = self.get_response(dialog=dialog, data=data, num_retries=num_retries) if resp: - match, score = match_one(resp, options) - if score < min_conf: - if self.voc_match(resp, 'last'): - resp = options[-1] - else: - num = extract_number(resp, ordinals=True, lang=self.lang) + engine = self._get_selection_engine() + if engine is not None: + try: + resp = engine.match_option(utterance=resp, options=options, lang=self.lang) + except Exception as e: + LOG.error(f"OptionMatcher plugin failed: {e}") resp = None - if num and num <= len(options): - resp = options[num - 1] - else: - resp = match return resp def voc_list(self, voc_filename: str, diff --git a/pyproject.toml b/pyproject.toml index 420ffda5..4654a2f6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,7 @@ dependencies = [ "ovos_bus_client>=1.3.8a1,<2.0.0", "ovos-config>=0.0.12,<3.0.0", "ovos-solver-yes-no-plugin>=0.0.1,<1.0.0", + "ovos-option-matcher-fuzzy-plugin>=0.0.1,<1.0.0", "ovos-number-parser>=0.0.1,<1.0.0", "rapidfuzz", "langcodes", diff --git a/test/unittests/test_skill_interaction.py b/test/unittests/test_skill_interaction.py new file mode 100644 index 00000000..d8eabe01 --- /dev/null +++ b/test/unittests/test_skill_interaction.py @@ -0,0 +1,264 @@ +# Copyright 2024 Mycroft AI Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for OVOSSkill.ask_yesno and ask_selection agent-plugin integration.""" +import unittest +from unittest.mock import MagicMock, patch + + +def _make_skill(settings=None, config_skills=None): + """Build a minimal duck-typed object for testing OVOSSkill helper methods.""" + from ovos_workshop.skills.ovos import OVOSSkill + import types + + # Bind the methods under test onto a plain object to avoid OVOSSkill.__init__ + skill = MagicMock(spec=object) + skill.settings = settings or {} + skill.config_core = {"skills": config_skills or {}} + skill.lang = "en-us" + + skill._get_yesno_engine = types.MethodType(OVOSSkill._get_yesno_engine, skill) + skill._get_selection_engine = types.MethodType(OVOSSkill._get_selection_engine, skill) + skill.ask_yesno = types.MethodType(OVOSSkill.ask_yesno, skill) + skill.ask_selection = types.MethodType(OVOSSkill.ask_selection, skill) + return skill + + +class TestGetYesnoEngine(unittest.TestCase): + """Tests for OVOSSkill._get_yesno_engine().""" + + def test_no_plugin_configured_returns_none(self): + skill = _make_skill() + self.assertIsNone(skill._get_yesno_engine()) + + def test_config_core_plugin_loaded(self): + skill = _make_skill(config_skills={"ask_yesno_plugin": "fake-yesno-plugin"}) + mock_cls = MagicMock() + mock_instance = MagicMock() + mock_cls.return_value = mock_instance + with patch("ovos_plugin_manager.agents.load_yesno_plugin", return_value=mock_cls): + engine = skill._get_yesno_engine() + self.assertIs(engine, mock_instance) + + def test_settings_overrides_config_core(self): + skill = _make_skill( + settings={"ask_yesno_plugin": "settings-plugin"}, + config_skills={"ask_yesno_plugin": "config-plugin"}, + ) + mock_cls = MagicMock() + with patch("ovos_plugin_manager.agents.load_yesno_plugin", return_value=mock_cls) as mock_load: + skill._get_yesno_engine() + mock_load.assert_called_once_with("settings-plugin") + + def test_plugin_load_failure_returns_none(self): + skill = _make_skill(config_skills={"ask_yesno_plugin": "bad-plugin"}) + with patch("ovos_plugin_manager.agents.load_yesno_plugin", side_effect=Exception("oops")): + engine = skill._get_yesno_engine() + self.assertIsNone(engine) + + def test_engine_cached_across_calls(self): + skill = _make_skill(config_skills={"ask_yesno_plugin": "fake-plugin"}) + mock_cls = MagicMock() + with patch("ovos_plugin_manager.agents.load_yesno_plugin", return_value=mock_cls) as mock_load: + skill._get_yesno_engine() + skill._get_yesno_engine() + mock_load.assert_called_once() + + +class TestGetSelectionEngine(unittest.TestCase): + """Tests for OVOSSkill._get_selection_engine().""" + + def test_no_config_defaults_to_fuzzy_plugin(self): + """When no plugin is configured, ovos-option-matcher-fuzzy-plugin is used.""" + skill = _make_skill() + mock_cls = MagicMock() + with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls) as mock_load: + skill._get_selection_engine() + mock_load.assert_called_once_with("ovos-option-matcher-fuzzy-plugin") + + def test_config_core_plugin_loaded(self): + skill = _make_skill(config_skills={"ask_selection_plugin": "fake-option-matcher"}) + mock_cls = MagicMock() + mock_instance = MagicMock() + mock_cls.return_value = mock_instance + with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls): + engine = skill._get_selection_engine() + self.assertIs(engine, mock_instance) + + def test_settings_overrides_config_core(self): + skill = _make_skill( + settings={"ask_selection_plugin": "settings-option-matcher"}, + config_skills={"ask_selection_plugin": "config-option-matcher"}, + ) + mock_cls = MagicMock() + with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls) as mock_load: + skill._get_selection_engine() + mock_load.assert_called_once_with("settings-option-matcher") + + def test_plugin_load_failure_returns_none(self): + skill = _make_skill(config_skills={"ask_selection_plugin": "bad-plugin"}) + with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", side_effect=Exception("fail")): + engine = skill._get_selection_engine() + self.assertIsNone(engine) + + +class TestAskYesno(unittest.TestCase): + """Tests for OVOSSkill.ask_yesno().""" + + def _make_skill_with_response(self, response, settings=None, config_skills=None): + skill = _make_skill(settings=settings, config_skills=config_skills) + skill.get_response = MagicMock(return_value=response) + return skill + + def test_no_plugin_uses_yesno_solver_yes(self): + skill = self._make_skill_with_response("yeah sure") + with patch("ovos_workshop.skills.ovos.YesNoSolver") as mock_solver_cls: + mock_solver = MagicMock() + mock_solver.match_yes_or_no.return_value = True + mock_solver_cls.return_value = mock_solver + result = skill.ask_yesno("Do you want tea?") + self.assertEqual(result, "yes") + + def test_no_plugin_uses_yesno_solver_no(self): + skill = self._make_skill_with_response("nope") + with patch("ovos_workshop.skills.ovos.YesNoSolver") as mock_solver_cls: + mock_solver = MagicMock() + mock_solver.match_yes_or_no.return_value = False + mock_solver_cls.return_value = mock_solver + result = skill.ask_yesno("Do you want tea?") + self.assertEqual(result, "no") + + def test_no_plugin_unmatched_returns_raw_resp(self): + skill = self._make_skill_with_response("maybe later") + with patch("ovos_workshop.skills.ovos.YesNoSolver") as mock_solver_cls: + mock_solver = MagicMock() + mock_solver.match_yes_or_no.return_value = None + mock_solver_cls.return_value = mock_solver + result = skill.ask_yesno("Do you want tea?") + self.assertEqual(result, "maybe later") + + def test_no_plugin_none_response_returns_none(self): + skill = self._make_skill_with_response(None) + result = skill.ask_yesno("Do you want tea?") + self.assertIsNone(result) + + def test_plugin_configured_calls_engine(self): + skill = self._make_skill_with_response("yes please", + config_skills={"ask_yesno_plugin": "fake-plugin"}) + mock_engine = MagicMock() + mock_engine.yes_or_no.return_value = True + with patch.object(skill, "_get_yesno_engine", return_value=mock_engine): + result = skill.ask_yesno("Do you want tea?") + mock_engine.yes_or_no.assert_called_once_with( + question="Do you want tea?", response="yes please", lang="en-us" + ) + self.assertEqual(result, "yes") + + def test_plugin_configured_no_response(self): + skill = self._make_skill_with_response(None, + config_skills={"ask_yesno_plugin": "fake-plugin"}) + mock_engine = MagicMock() + with patch.object(skill, "_get_yesno_engine", return_value=mock_engine): + result = skill.ask_yesno("Do you want tea?") + mock_engine.yes_or_no.assert_not_called() + self.assertIsNone(result) + + def test_plugin_returns_false_maps_to_no(self): + skill = self._make_skill_with_response("no way", + config_skills={"ask_yesno_plugin": "fake-plugin"}) + mock_engine = MagicMock() + mock_engine.yes_or_no.return_value = False + with patch.object(skill, "_get_yesno_engine", return_value=mock_engine): + result = skill.ask_yesno("Do you want tea?") + self.assertEqual(result, "no") + + +class TestAskSelection(unittest.TestCase): + """Tests for OVOSSkill.ask_selection().""" + + def _make_selection_skill(self, response, settings=None, config_skills=None): + skill = _make_skill(settings=settings, config_skills=config_skills) + skill.get_response = MagicMock(return_value=response) + skill.speak = MagicMock() + return skill + + def test_plugin_called_with_response(self): + """Default fuzzy plugin (or any configured plugin) receives the user response.""" + skill = self._make_selection_skill("beta") + options = ["alpha", "beta", "gamma"] + mock_engine = MagicMock() + mock_engine.match_option.return_value = "beta" + with patch.object(skill, "_get_selection_engine", return_value=mock_engine): + result = skill.ask_selection(options, numeric=True) + mock_engine.match_option.assert_called_once_with( + utterance="beta", options=options, lang="en-us" + ) + self.assertEqual(result, "beta") + + def test_plugin_runtime_failure_returns_none(self): + """If the engine raises, ask_selection returns None rather than crashing.""" + skill = self._make_selection_skill("alpha") + options = ["alpha", "beta", "gamma"] + mock_engine = MagicMock() + mock_engine.match_option.side_effect = RuntimeError("model error") + with patch.object(skill, "_get_selection_engine", return_value=mock_engine): + result = skill.ask_selection(options, numeric=True) + self.assertIsNone(result) + + def test_no_engine_no_response_returns_none(self): + """If engine load fails and user gives no response, return None.""" + skill = self._make_selection_skill(None) + options = ["alpha", "beta"] + with patch.object(skill, "_get_selection_engine", return_value=None): + result = skill.ask_selection(options, numeric=True) + self.assertIsNone(result) + + def test_no_response_returns_none(self): + skill = self._make_selection_skill(None) + options = ["alpha", "beta"] + result = skill.ask_selection(options, numeric=True) + self.assertIsNone(result) + + def test_single_option_returns_immediately(self): + skill = self._make_selection_skill(None) + result = skill.ask_selection(["only"], numeric=True) + self.assertEqual(result, "only") + skill.speak.assert_not_called() + + def test_empty_options_returns_none(self): + skill = self._make_selection_skill(None) + result = skill.ask_selection([]) + self.assertIsNone(result) + + def test_invalid_options_raises(self): + skill = self._make_selection_skill(None) + with self.assertRaises(ValueError): + skill.ask_selection("not a list") + + def test_settings_plugin_overrides_default(self): + """settings.json ask_selection_plugin takes precedence over the fuzzy default.""" + skill = self._make_selection_skill( + "first", settings={"ask_selection_plugin": "my-custom-option-matcher"} + ) + options = ["alpha", "beta", "gamma"] + mock_cls = MagicMock() + mock_instance = MagicMock() + mock_instance.match_option.return_value = "alpha" + mock_cls.return_value = mock_instance + with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls) as mock_load: + skill.ask_selection(options, numeric=True) + mock_load.assert_called_once_with("my-custom-option-matcher") + + +if __name__ == "__main__": + unittest.main() From d087e639266c22b5a0fd8417f12254834532c056 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 20:13:23 +0100 Subject: [PATCH 02/22] fix: default plugins --- ovos_workshop/skills/ovos.py | 36 +++++++++++++++++------------------ pyproject.toml | 1 + requirements/requirements.txt | 1 + 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 013ff6f8..2c2c78fb 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -63,6 +63,11 @@ from ovos_workshop.resource_files import ResourceFile, CoreResources, find_resource, SkillResources from ovos_workshop.settings import PrivateSettings from ovos_workshop.skills.util import join_word_list, simple_trace +from ovos_plugin_manager.agents import load_yesno_plugin, load_option_matcher_plugin +from ovos_plugin_manager.templates.agents import YesNoEngine, OptionMatcherEngine +from ovos_yes_no_solver import YesNoSolver +from ovos_option_matcher_fuzzy import FuzzyOptionMatcherPlugin + class OVOSSkill: @@ -1928,28 +1933,26 @@ def acknowledge(self): 'snd/acknowledge.mp3') self.play_audio(audio_file, instant=True) - def _get_yesno_engine(self) -> Optional[object]: + def _get_yesno_engine(self) -> YesNoEngine: """Load the configured YesNoEngine plugin, with per-skill override support. Checks settings.json first, then mycroft.conf skills.ask_yesno_plugin. Returns None if no plugin is configured, preserving built-in fallback behavior. """ plugin_name = (self.settings.get("ask_yesno_plugin") or - self.config_core.get("skills", {}).get("ask_yesno_plugin")) - if not plugin_name: - return None + self.config_core.get("skills", {}).get("ask_yesno_plugin") or + "ovos-solver-yes-no-plugin") cache_key = f"__yesno_engine_{plugin_name}" if not hasattr(self, cache_key): try: - from ovos_plugin_manager.agents import load_yesno_plugin cls = load_yesno_plugin(plugin_name) setattr(self, cache_key, cls()) except Exception as e: LOG.error(f"Failed to load YesNo plugin '{plugin_name}': {e}") setattr(self, cache_key, None) - return getattr(self, cache_key) + return getattr(self, cache_key) or YesNoSolver() - def _get_selection_engine(self) -> Optional[object]: + def _get_selection_engine(self) -> OptionMatcherEngine: """Load the configured OptionMatcherEngine plugin, with per-skill override support. Checks settings.json first, then mycroft.conf skills.ask_selection_plugin, @@ -1961,13 +1964,12 @@ def _get_selection_engine(self) -> Optional[object]: cache_key = f"__selection_engine_{plugin_name}" if not hasattr(self, cache_key): try: - from ovos_plugin_manager.agents import load_option_matcher_plugin cls = load_option_matcher_plugin(plugin_name) setattr(self, cache_key, cls()) except Exception as e: LOG.error(f"Failed to load selection plugin '{plugin_name}': {e}") setattr(self, cache_key, None) - return getattr(self, cache_key) + return getattr(self, cache_key) or FuzzyOptionMatcherPlugin() def ask_yesno(self, prompt: str, data: Optional[dict] = None) -> Optional[str]: @@ -1981,10 +1983,7 @@ def ask_yesno(self, prompt: str, """ resp = self.get_response(dialog=prompt, data=data) engine = self._get_yesno_engine() - if engine is not None: - answer = engine.yes_or_no(question=prompt, response=resp, lang=self.lang) if resp else None - else: - answer = YesNoSolver().match_yes_or_no(resp, lang=self.lang) if resp else resp + answer = engine.yes_or_no(question=prompt, response=resp, lang=self.lang) if resp else None if answer is True: return "yes" elif answer is False: @@ -2036,12 +2035,11 @@ def ask_selection(self, options: List[str], dialog: str = '', if resp: engine = self._get_selection_engine() - if engine is not None: - try: - resp = engine.match_option(utterance=resp, options=options, lang=self.lang) - except Exception as e: - LOG.error(f"OptionMatcher plugin failed: {e}") - resp = None + try: + resp = engine.match_option(utterance=resp, options=options, lang=self.lang) + except Exception as e: + LOG.error(f"OptionMatcher plugin failed: {e}") + resp = None return resp def voc_list(self, voc_filename: str, diff --git a/pyproject.toml b/pyproject.toml index 4654a2f6..b994ff9b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,6 +17,7 @@ dependencies = [ "ovos-solver-yes-no-plugin>=0.0.1,<1.0.0", "ovos-option-matcher-fuzzy-plugin>=0.0.1,<1.0.0", "ovos-number-parser>=0.0.1,<1.0.0", + "ovos-plugin-manager>=2.4.0a1,<3.0.0", "rapidfuzz", "langcodes", "padacioso>=1.0.0, <2.0.0", diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 8691764c..b311c93b 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -3,6 +3,7 @@ ovos_bus_client>=1.3.8a1,<2.0.0 ovos-config>=0.0.12,<3.0.0 ovos-solver-yes-no-plugin>=0.0.1,<1.0.0 ovos-number-parser>=0.0.1,<1.0.0 +ovos-plugin-manager>=2.4.0a1,<3.0.0 rapidfuzz langcodes padacioso>=1.0.0, <2.0.0 \ No newline at end of file From c189de55558d5bcd4615cefacb1ea7b40fc89efc Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 22:20:00 +0100 Subject: [PATCH 03/22] fix: dependencies --- ovos_workshop/skills/ovos.py | 28 ++++++++++++---------------- pyproject.toml | 2 +- requirements/requirements.txt | 3 ++- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 2c2c78fb..e5591811 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -13,7 +13,6 @@ # limitations under the License. import binascii import datetime -import json import os import re import shutil @@ -29,45 +28,42 @@ from typing import Dict, Callable, List, Optional, Union from json_database import JsonStorage -from ovos_config.config import Configuration -from ovos_config.locations import get_xdg_cache_save_path -from ovos_config.locations import get_xdg_config_save_path -from ovos_number_parser import pronounce_number, extract_number -from ovos_yes_no_solver import YesNoSolver - from ovos_bus_client import MessageBusClient from ovos_bus_client.apis.enclosure import EnclosureAPI +from ovos_bus_client.apis.events import EventSchedulerInterface from ovos_bus_client.apis.gui import GUIInterface from ovos_bus_client.apis.ocp import OCPInterface from ovos_bus_client.message import Message, dig_for_message from ovos_bus_client.session import SessionManager, Session from ovos_bus_client.util import get_message_lang +from ovos_config.config import Configuration +from ovos_config.locations import get_xdg_cache_save_path +from ovos_config.locations import get_xdg_config_save_path +from ovos_number_parser import pronounce_number +from ovos_option_matcher_fuzzy import FuzzyOptionMatcherPlugin +from ovos_plugin_manager.agents import load_yesno_plugin, load_option_matcher_plugin from ovos_plugin_manager.language import OVOSLangTranslationFactory, OVOSLangDetectionFactory +from ovos_plugin_manager.templates.agents import YesNoEngine, OptionMatcherEngine from ovos_utils import camel_case_split, classproperty from ovos_utils.dialog import MustacheDialogRenderer -from ovos_bus_client.apis.events import EventSchedulerInterface from ovos_utils.events import EventContainer, get_handler_name, create_wrapper from ovos_utils.file_utils import FileWatcher from ovos_utils.gui import get_ui_directories from ovos_utils.json_helper import merge_dict from ovos_utils.lang import standardize_lang_tag from ovos_utils.log import LOG -from ovos_utils.parse import match_one from ovos_utils.process_utils import ProcessStatus, StatusCallbackMap, RuntimeRequirements from ovos_utils.skills import get_non_properties from ovos_utils.text_utils import remove_accents_and_punct +from ovos_yes_no import HeuristicYesNoEngine + from ovos_workshop.decorators.killable import AbortEvent, killable_event, AbortQuestion from ovos_workshop.decorators.layers import IntentLayers from ovos_workshop.filesystem import FileSystemAccess from ovos_workshop.intents import IntentBuilder, Intent, munge_regex, munge_intent_parser, IntentServiceInterface -from ovos_workshop.resource_files import ResourceFile, CoreResources, find_resource, SkillResources +from ovos_workshop.resource_files import ResourceFile, find_resource, SkillResources from ovos_workshop.settings import PrivateSettings from ovos_workshop.skills.util import join_word_list, simple_trace -from ovos_plugin_manager.agents import load_yesno_plugin, load_option_matcher_plugin -from ovos_plugin_manager.templates.agents import YesNoEngine, OptionMatcherEngine -from ovos_yes_no_solver import YesNoSolver -from ovos_option_matcher_fuzzy import FuzzyOptionMatcherPlugin - class OVOSSkill: @@ -1950,7 +1946,7 @@ def _get_yesno_engine(self) -> YesNoEngine: except Exception as e: LOG.error(f"Failed to load YesNo plugin '{plugin_name}': {e}") setattr(self, cache_key, None) - return getattr(self, cache_key) or YesNoSolver() + return getattr(self, cache_key) or HeuristicYesNoEngine() def _get_selection_engine(self) -> OptionMatcherEngine: """Load the configured OptionMatcherEngine plugin, with per-skill override support. diff --git a/pyproject.toml b/pyproject.toml index b994ff9b..16878da9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ dependencies = [ "ovos-utils>= 0.7.0,<1.0.0", "ovos_bus_client>=1.3.8a1,<2.0.0", "ovos-config>=0.0.12,<3.0.0", - "ovos-solver-yes-no-plugin>=0.0.1,<1.0.0", + "ovos-yes-no-plugin>=0.3.0,<1.0.0", "ovos-option-matcher-fuzzy-plugin>=0.0.1,<1.0.0", "ovos-number-parser>=0.0.1,<1.0.0", "ovos-plugin-manager>=2.4.0a1,<3.0.0", diff --git a/requirements/requirements.txt b/requirements/requirements.txt index b311c93b..810d9cd9 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -1,7 +1,8 @@ ovos-utils>= 0.7.0,<1.0.0 ovos_bus_client>=1.3.8a1,<2.0.0 ovos-config>=0.0.12,<3.0.0 -ovos-solver-yes-no-plugin>=0.0.1,<1.0.0 +ovos-yes-no-plugin>=0.3.0,<1.0.0 +ovos-option-matcher-fuzzy-plugin>=0.0.1,<1.0.0 ovos-number-parser>=0.0.1,<1.0.0 ovos-plugin-manager>=2.4.0a1,<3.0.0 rapidfuzz From 268c197f2535cc6a450bddbfd96a5b1fb27df722 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 23:21:31 +0100 Subject: [PATCH 04/22] chore: drop requirements.txt, deps now in pyproject.toml Co-Authored-By: Claude Sonnet 4.6 --- requirements/requirements.txt | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 requirements/requirements.txt diff --git a/requirements/requirements.txt b/requirements/requirements.txt deleted file mode 100644 index 810d9cd9..00000000 --- a/requirements/requirements.txt +++ /dev/null @@ -1,10 +0,0 @@ -ovos-utils>= 0.7.0,<1.0.0 -ovos_bus_client>=1.3.8a1,<2.0.0 -ovos-config>=0.0.12,<3.0.0 -ovos-yes-no-plugin>=0.3.0,<1.0.0 -ovos-option-matcher-fuzzy-plugin>=0.0.1,<1.0.0 -ovos-number-parser>=0.0.1,<1.0.0 -ovos-plugin-manager>=2.4.0a1,<3.0.0 -rapidfuzz -langcodes -padacioso>=1.0.0, <2.0.0 \ No newline at end of file From a89c7965a5cac6fc0bc6f17dc31d76a67dab6fe9 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 23:27:28 +0100 Subject: [PATCH 05/22] fix: correct test patch paths and expected fallback types - patch load_yesno_plugin/load_option_matcher_plugin at ovos_workshop.skills.ovos (not ovos_plugin_manager.agents) since they are imported into the module - _get_yesno_engine falls back to HeuristicYesNoEngine, not None - _get_selection_engine falls back to FuzzyOptionMatcherPlugin, not None - replace YesNoSolver references with patch.object on _get_yesno_engine Co-Authored-By: Claude Sonnet 4.6 --- test/unittests/test_skill_interaction.py | 62 +++++++++++++----------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/test/unittests/test_skill_interaction.py b/test/unittests/test_skill_interaction.py index d8eabe01..73ddeb6f 100644 --- a/test/unittests/test_skill_interaction.py +++ b/test/unittests/test_skill_interaction.py @@ -37,16 +37,19 @@ def _make_skill(settings=None, config_skills=None): class TestGetYesnoEngine(unittest.TestCase): """Tests for OVOSSkill._get_yesno_engine().""" - def test_no_plugin_configured_returns_none(self): + def test_no_plugin_configured_returns_heuristic(self): + """With no plugin configured, falls back to HeuristicYesNoEngine.""" + from ovos_yes_no import HeuristicYesNoEngine skill = _make_skill() - self.assertIsNone(skill._get_yesno_engine()) + engine = skill._get_yesno_engine() + self.assertIsInstance(engine, HeuristicYesNoEngine) def test_config_core_plugin_loaded(self): skill = _make_skill(config_skills={"ask_yesno_plugin": "fake-yesno-plugin"}) mock_cls = MagicMock() mock_instance = MagicMock() mock_cls.return_value = mock_instance - with patch("ovos_plugin_manager.agents.load_yesno_plugin", return_value=mock_cls): + with patch("ovos_workshop.skills.ovos.load_yesno_plugin", return_value=mock_cls): engine = skill._get_yesno_engine() self.assertIs(engine, mock_instance) @@ -56,20 +59,22 @@ def test_settings_overrides_config_core(self): config_skills={"ask_yesno_plugin": "config-plugin"}, ) mock_cls = MagicMock() - with patch("ovos_plugin_manager.agents.load_yesno_plugin", return_value=mock_cls) as mock_load: + with patch("ovos_workshop.skills.ovos.load_yesno_plugin", return_value=mock_cls) as mock_load: skill._get_yesno_engine() mock_load.assert_called_once_with("settings-plugin") - def test_plugin_load_failure_returns_none(self): + def test_plugin_load_failure_returns_heuristic(self): + """On load failure, falls back to HeuristicYesNoEngine.""" + from ovos_yes_no import HeuristicYesNoEngine skill = _make_skill(config_skills={"ask_yesno_plugin": "bad-plugin"}) - with patch("ovos_plugin_manager.agents.load_yesno_plugin", side_effect=Exception("oops")): + with patch("ovos_workshop.skills.ovos.load_yesno_plugin", side_effect=Exception("oops")): engine = skill._get_yesno_engine() - self.assertIsNone(engine) + self.assertIsInstance(engine, HeuristicYesNoEngine) def test_engine_cached_across_calls(self): skill = _make_skill(config_skills={"ask_yesno_plugin": "fake-plugin"}) mock_cls = MagicMock() - with patch("ovos_plugin_manager.agents.load_yesno_plugin", return_value=mock_cls) as mock_load: + with patch("ovos_workshop.skills.ovos.load_yesno_plugin", return_value=mock_cls) as mock_load: skill._get_yesno_engine() skill._get_yesno_engine() mock_load.assert_called_once() @@ -82,7 +87,7 @@ def test_no_config_defaults_to_fuzzy_plugin(self): """When no plugin is configured, ovos-option-matcher-fuzzy-plugin is used.""" skill = _make_skill() mock_cls = MagicMock() - with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls) as mock_load: + with patch("ovos_workshop.skills.ovos.load_option_matcher_plugin", return_value=mock_cls) as mock_load: skill._get_selection_engine() mock_load.assert_called_once_with("ovos-option-matcher-fuzzy-plugin") @@ -91,7 +96,7 @@ def test_config_core_plugin_loaded(self): mock_cls = MagicMock() mock_instance = MagicMock() mock_cls.return_value = mock_instance - with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls): + with patch("ovos_workshop.skills.ovos.load_option_matcher_plugin", return_value=mock_cls): engine = skill._get_selection_engine() self.assertIs(engine, mock_instance) @@ -101,15 +106,17 @@ def test_settings_overrides_config_core(self): config_skills={"ask_selection_plugin": "config-option-matcher"}, ) mock_cls = MagicMock() - with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls) as mock_load: + with patch("ovos_workshop.skills.ovos.load_option_matcher_plugin", return_value=mock_cls) as mock_load: skill._get_selection_engine() mock_load.assert_called_once_with("settings-option-matcher") - def test_plugin_load_failure_returns_none(self): + def test_plugin_load_failure_returns_fuzzy_fallback(self): + """On load failure, falls back to FuzzyOptionMatcherPlugin.""" + from ovos_option_matcher_fuzzy import FuzzyOptionMatcherPlugin skill = _make_skill(config_skills={"ask_selection_plugin": "bad-plugin"}) - with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", side_effect=Exception("fail")): + with patch("ovos_workshop.skills.ovos.load_option_matcher_plugin", side_effect=Exception("fail")): engine = skill._get_selection_engine() - self.assertIsNone(engine) + self.assertIsInstance(engine, FuzzyOptionMatcherPlugin) class TestAskYesno(unittest.TestCase): @@ -120,30 +127,27 @@ def _make_skill_with_response(self, response, settings=None, config_skills=None) skill.get_response = MagicMock(return_value=response) return skill - def test_no_plugin_uses_yesno_solver_yes(self): + def test_no_plugin_uses_heuristic_engine_yes(self): skill = self._make_skill_with_response("yeah sure") - with patch("ovos_workshop.skills.ovos.YesNoSolver") as mock_solver_cls: - mock_solver = MagicMock() - mock_solver.match_yes_or_no.return_value = True - mock_solver_cls.return_value = mock_solver + mock_engine = MagicMock() + mock_engine.yes_or_no.return_value = True + with patch.object(skill, "_get_yesno_engine", return_value=mock_engine): result = skill.ask_yesno("Do you want tea?") self.assertEqual(result, "yes") - def test_no_plugin_uses_yesno_solver_no(self): + def test_no_plugin_uses_heuristic_engine_no(self): skill = self._make_skill_with_response("nope") - with patch("ovos_workshop.skills.ovos.YesNoSolver") as mock_solver_cls: - mock_solver = MagicMock() - mock_solver.match_yes_or_no.return_value = False - mock_solver_cls.return_value = mock_solver + mock_engine = MagicMock() + mock_engine.yes_or_no.return_value = False + with patch.object(skill, "_get_yesno_engine", return_value=mock_engine): result = skill.ask_yesno("Do you want tea?") self.assertEqual(result, "no") def test_no_plugin_unmatched_returns_raw_resp(self): skill = self._make_skill_with_response("maybe later") - with patch("ovos_workshop.skills.ovos.YesNoSolver") as mock_solver_cls: - mock_solver = MagicMock() - mock_solver.match_yes_or_no.return_value = None - mock_solver_cls.return_value = mock_solver + mock_engine = MagicMock() + mock_engine.yes_or_no.return_value = None + with patch.object(skill, "_get_yesno_engine", return_value=mock_engine): result = skill.ask_yesno("Do you want tea?") self.assertEqual(result, "maybe later") @@ -255,7 +259,7 @@ def test_settings_plugin_overrides_default(self): mock_instance = MagicMock() mock_instance.match_option.return_value = "alpha" mock_cls.return_value = mock_instance - with patch("ovos_plugin_manager.agents.load_option_matcher_plugin", return_value=mock_cls) as mock_load: + with patch("ovos_workshop.skills.ovos.load_option_matcher_plugin", return_value=mock_cls) as mock_load: skill.ask_selection(options, numeric=True) mock_load.assert_called_once_with("my-custom-option-matcher") From e838787f55b9e5df95920d3ddd9dc458a8490223 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 23:27:59 +0100 Subject: [PATCH 06/22] chore: drop duplicate unit_tests workflow build_tests already runs the same test path; unit_tests only differed by adding audio system deps not needed for these tests Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/test.yml | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index b5849b93..00000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,15 +0,0 @@ -name: Run Unit Tests -on: - push: - branches: [dev] - pull_request: - branches: [dev] - workflow_dispatch: - -jobs: - unit_tests: - uses: OpenVoiceOS/gh-automations/.github/workflows/build-tests.yml@dev - secrets: inherit - with: - test_path: 'test/unittests' - system_deps: 'libssl-dev libfann-dev portaudio19-dev libpulse-dev' From 5da520d0d1b90b0aaf5dff8a3e6e734c68cd5836 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 23:56:03 +0100 Subject: [PATCH 07/22] test: add ask_yesno and ask_selection tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test_base.py: fill in TODO stubs with mock-based tests on real OVOSSkill (yes/no/unmatched/timeout for ask_yesno; empty/single/invalid/fuzzy/timeout for ask_selection) - test_ask_e2e.py: ovoscope end-to-end tests — inline AskYesNoSkill and AskSelectionSkill loaded via get_minicroft; user response injected via {skill_id}.converse.get_response after hearing speak; skipped when ovoscope/ovos-core are not installed Co-Authored-By: Claude Sonnet 4.6 --- test/unittests/skills/test_base.py | 46 ++++++- test/unittests/test_ask_e2e.py | 203 +++++++++++++++++++++++++++++ 2 files changed, 245 insertions(+), 4 deletions(-) create mode 100644 test/unittests/test_ask_e2e.py diff --git a/test/unittests/skills/test_base.py b/test/unittests/skills/test_base.py index 2f4534af..7d07c166 100644 --- a/test/unittests/skills/test_base.py +++ b/test/unittests/skills/test_base.py @@ -228,12 +228,50 @@ def test_get_intro_message(self): # TODO port get_response methods per #69 def test_ask_yesno(self): - # TODO - pass + from unittest.mock import patch + + # "yes" response -> "yes" + with patch.object(self.skill, 'get_response', return_value='yes'): + self.assertEqual(self.skill.ask_yesno('do you want tea'), 'yes') + + # "nope" response -> "no" + with patch.object(self.skill, 'get_response', return_value='nope'): + self.assertEqual(self.skill.ask_yesno('do you want tea'), 'no') + + # "maybe" -> not matched, raw response returned + with patch.object(self.skill, 'get_response', return_value='maybe'): + self.assertEqual(self.skill.ask_yesno('do you want tea'), 'maybe') + + # None response (timeout) -> None + with patch.object(self.skill, 'get_response', return_value=None): + self.assertIsNone(self.skill.ask_yesno('do you want tea')) def test_ask_selection(self): - # TODO - pass + from unittest.mock import patch + + options = ['alpha', 'beta', 'gamma'] + + # empty list -> None + self.assertIsNone(self.skill.ask_selection([])) + + # single option -> returned immediately without prompting + with patch.object(self.skill, 'speak', wraps=self.skill.speak) as mock_speak: + result = self.skill.ask_selection(['only']) + self.assertEqual(result, 'only') + + # invalid type -> ValueError + with self.assertRaises(ValueError): + self.skill.ask_selection('not a list') + + # fuzzy match "beta" -> "beta" + with patch.object(self.skill, 'get_response', return_value='beta'): + result = self.skill.ask_selection(options, numeric=True) + self.assertEqual(result, 'beta') + + # no response (timeout) -> None + with patch.object(self.skill, 'get_response', return_value=None): + result = self.skill.ask_selection(options, numeric=True) + self.assertIsNone(result) def test_voc_list(self): # TODO diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py new file mode 100644 index 00000000..d218d10f --- /dev/null +++ b/test/unittests/test_ask_e2e.py @@ -0,0 +1,203 @@ +# Copyright 2026 OpenVoiceOS +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Ovoscope end-to-end tests for ask_yesno and ask_selection. + +These tests require ovoscope + ovos-core and are skipped when those +packages are not installed (e.g. in the default CI build matrix). +To run locally: + pip install ovoscope ovos-core + pytest test/unittests/test_ask_e2e.py -v +""" +import threading +import unittest + +import pytest + +ovoscope = pytest.importorskip("ovoscope") + +from ovos_bus_client.message import Message +from ovos_bus_client.session import SessionManager, Session +from ovos_utils.log import LOG +from ovos_workshop.skills.ovos import OVOSSkill + +from ovoscope import get_minicroft, CaptureSession + +# --------------------------------------------------------------------------- +# Shared skill IDs +# --------------------------------------------------------------------------- +YESNO_SKILL_ID = "test.ask.yesno.skill" +SELECT_SKILL_ID = "test.ask.selection.skill" + + +# --------------------------------------------------------------------------- +# Inline test skills +# --------------------------------------------------------------------------- + +class AskYesNoSkill(OVOSSkill): + """Handles 'test.ask.yesno': calls ask_yesno and emits the result.""" + + def initialize(self): + self.add_event("test.ask.yesno", self.handle_yesno) + + def handle_yesno(self, message: Message): + answer = self.ask_yesno("do you want tea", message=message) + self.bus.emit(message.forward("test.yesno.result", {"answer": answer})) + self.bus.emit(message.forward("ovos.utterance.handled")) + + +class AskSelectionSkill(OVOSSkill): + """Handles 'test.ask.selection': calls ask_selection and emits the result.""" + + def initialize(self): + self.add_event("test.ask.selection", self.handle_selection) + + def handle_selection(self, message: Message): + options = message.data.get("options", ["alpha", "beta", "gamma"]) + answer = self.ask_selection(options, numeric=True, message=message) + self.bus.emit(message.forward("test.selection.result", {"answer": answer})) + self.bus.emit(message.forward("ovos.utterance.handled")) + + +# --------------------------------------------------------------------------- +# Helper — inject a user response after the skill starts listening +# --------------------------------------------------------------------------- + +def _inject_response_after_speak(mc, skill_id: str, utterance: str, + session: Session, delay: float = 0.3): + """Wait for the skill to speak (prompt), then inject user utterance. + + Emits directly to ``{skill_id}.converse.get_response`` which is the + internal bus event that ``_wait_response`` reads from. + """ + spoken = threading.Event() + + def on_speak(msg: str): + spoken.set() + + mc.bus.on("speak", on_speak) + + def _inject(): + spoken.wait(timeout=10) + mc.bus.remove("speak", on_speak) + import time; time.sleep(delay) + mc.bus.emit(Message( + f"{skill_id}.converse.get_response", + {"utterances": [utterance], "lang": "en-us"}, + {"session": session.serialize(), "skill_id": skill_id}, + )) + + t = threading.Thread(target=_inject, daemon=True) + t.start() + return t + + +def _make_trigger(msg_type: str, skill_id: str, + data: dict = None, session_id: str = "e2e-test") -> Message: + sess = Session(session_id) + sess.lang = "en-us" + return Message(msg_type, data or {}, + {"session": sess.serialize(), + "skill_id": skill_id, + "source": "test", "destination": skill_id}) + + +# --------------------------------------------------------------------------- +# Tests: ask_yesno +# --------------------------------------------------------------------------- + +class TestAskYesnoE2E(unittest.TestCase): + + @classmethod + def setUpClass(cls): + LOG.set_level("ERROR") + cls.mc = get_minicroft([YESNO_SKILL_ID], + extra_skills={YESNO_SKILL_ID: AskYesNoSkill}) + + @classmethod + def tearDownClass(cls): + cls.mc.stop() + + def _run(self, user_says: str, session_id: str = "e2e-yesno") -> list: + """Trigger ask_yesno, inject a user reply, return captured messages.""" + trigger = _make_trigger("test.ask.yesno", YESNO_SKILL_ID, + session_id=session_id) + sess = SessionManager.get(trigger) + _inject_response_after_speak(self.mc, YESNO_SKILL_ID, + user_says, sess) + cap = CaptureSession(self.mc) + cap.capture(trigger, timeout=15) + return cap.finish() + + def test_yes_response(self): + msgs = self._run("yes", session_id="e2e-yesno-yes") + results = [m for m in msgs if m.msg_type == "test.yesno.result"] + self.assertTrue(results, "test.yesno.result not emitted") + self.assertEqual(results[0].data["answer"], "yes") + + def test_no_response(self): + msgs = self._run("nope", session_id="e2e-yesno-no") + results = [m for m in msgs if m.msg_type == "test.yesno.result"] + self.assertTrue(results, "test.yesno.result not emitted") + self.assertEqual(results[0].data["answer"], "no") + + def test_unmatched_response_returns_raw(self): + msgs = self._run("maybe later", session_id="e2e-yesno-maybe") + results = [m for m in msgs if m.msg_type == "test.yesno.result"] + self.assertTrue(results, "test.yesno.result not emitted") + self.assertEqual(results[0].data["answer"], "maybe later") + + +# --------------------------------------------------------------------------- +# Tests: ask_selection +# --------------------------------------------------------------------------- + +class TestAskSelectionE2E(unittest.TestCase): + + @classmethod + def setUpClass(cls): + LOG.set_level("ERROR") + cls.mc = get_minicroft([SELECT_SKILL_ID], + extra_skills={SELECT_SKILL_ID: AskSelectionSkill}) + + @classmethod + def tearDownClass(cls): + cls.mc.stop() + + def _run(self, user_says: str, options: list = None, + session_id: str = "e2e-select") -> list: + trigger = _make_trigger("test.ask.selection", SELECT_SKILL_ID, + data={"options": options or ["alpha", "beta", "gamma"]}, + session_id=session_id) + sess = SessionManager.get(trigger) + _inject_response_after_speak(self.mc, SELECT_SKILL_ID, + user_says, sess) + cap = CaptureSession(self.mc) + cap.capture(trigger, timeout=15) + return cap.finish() + + def test_fuzzy_match(self): + msgs = self._run("beta", session_id="e2e-select-beta") + results = [m for m in msgs if m.msg_type == "test.selection.result"] + self.assertTrue(results, "test.selection.result not emitted") + self.assertEqual(results[0].data["answer"], "beta") + + def test_first_option(self): + msgs = self._run("alpha", session_id="e2e-select-alpha") + results = [m for m in msgs if m.msg_type == "test.selection.result"] + self.assertTrue(results, "test.selection.result not emitted") + self.assertEqual(results[0].data["answer"], "alpha") + + +if __name__ == "__main__": + unittest.main() From d106049b06fceebe643dcf29f2ca36bd557bd954 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 8 Apr 2026 23:57:54 +0100 Subject: [PATCH 08/22] test: require ovoscope for e2e tests, add [test] extras MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove pytest.importorskip — e2e tests are mandatory, not optional - add [project.optional-dependencies] test group to pyproject.toml (ovos-core, ovoscope, pytest, pytest-cov, ovos-translate-server-plugin) - build_tests.yml: pass install_extras=test so CI installs test deps - keep requirements/test.txt in sync for local pip install -r workflows Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/build_tests.yml | 1 + pyproject.toml | 9 +++++++++ requirements/test.txt | 1 + test/unittests/test_ask_e2e.py | 13 +------------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index 514024e0..237096e4 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -12,3 +12,4 @@ jobs: secrets: inherit with: test_path: 'test/unittests' + install_extras: 'test' diff --git a/pyproject.toml b/pyproject.toml index 16878da9..3398479c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,15 @@ dependencies = [ "padacioso>=1.0.0, <2.0.0", ] +[project.optional-dependencies] +test = [ + "ovos-core>=0.0.8a50", + "ovoscope>=0.1.0", + "pytest", + "pytest-cov", + "ovos-translate-server-plugin", +] + [project.urls] Homepage = "https://github.com/OpenVoiceOS/OVOS-workshop" Repository = "https://github.com/OpenVoiceOS/OVOS-workshop" diff --git a/requirements/test.txt b/requirements/test.txt index 431c2ec4..1c04ab60 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,4 +1,5 @@ ovos-core>=0.0.8a50 +ovoscope>=0.1.0 pytest pytest-cov ovos-translate-server-plugin \ No newline at end of file diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py index d218d10f..d551a617 100644 --- a/test/unittests/test_ask_e2e.py +++ b/test/unittests/test_ask_e2e.py @@ -11,21 +11,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Ovoscope end-to-end tests for ask_yesno and ask_selection. - -These tests require ovoscope + ovos-core and are skipped when those -packages are not installed (e.g. in the default CI build matrix). -To run locally: - pip install ovoscope ovos-core - pytest test/unittests/test_ask_e2e.py -v -""" +"""Ovoscope end-to-end tests for ask_yesno and ask_selection.""" import threading import unittest -import pytest - -ovoscope = pytest.importorskip("ovoscope") - from ovos_bus_client.message import Message from ovos_bus_client.session import SessionManager, Session from ovos_utils.log import LOG From 97db28d7af888d3b69f8ecb70a785b06980f03a8 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 00:02:18 +0100 Subject: [PATCH 09/22] fix: remove unsupported message kwarg from ask_yesno/ask_selection calls Co-Authored-By: Claude Sonnet 4.6 --- test/unittests/test_ask_e2e.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py index d551a617..0d5cbc3a 100644 --- a/test/unittests/test_ask_e2e.py +++ b/test/unittests/test_ask_e2e.py @@ -40,7 +40,7 @@ def initialize(self): self.add_event("test.ask.yesno", self.handle_yesno) def handle_yesno(self, message: Message): - answer = self.ask_yesno("do you want tea", message=message) + answer = self.ask_yesno("do you want tea") self.bus.emit(message.forward("test.yesno.result", {"answer": answer})) self.bus.emit(message.forward("ovos.utterance.handled")) @@ -53,7 +53,7 @@ def initialize(self): def handle_selection(self, message: Message): options = message.data.get("options", ["alpha", "beta", "gamma"]) - answer = self.ask_selection(options, numeric=True, message=message) + answer = self.ask_selection(options, numeric=True) self.bus.emit(message.forward("test.selection.result", {"answer": answer})) self.bus.emit(message.forward("ovos.utterance.handled")) From 8608140ecd42a32487c390a1b05c506db64acc68 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 00:04:20 +0100 Subject: [PATCH 10/22] . --- test/unittests/test_skill_interaction.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unittests/test_skill_interaction.py b/test/unittests/test_skill_interaction.py index 73ddeb6f..b5f9c051 100644 --- a/test/unittests/test_skill_interaction.py +++ b/test/unittests/test_skill_interaction.py @@ -1,5 +1,3 @@ -# Copyright 2024 Mycroft AI Inc. -# # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at From 7c7a5a716c1a1a6afd91eb53a074ecb5a1a623ab Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 00:12:40 +0100 Subject: [PATCH 11/22] fix: forward min_conf to OptionMatcher engine via config dict The ask_selection min_conf parameter was documented but silently ignored. FuzzyOptionMatcherPlugin reads threshold from self.config["min_conf"], so set it on the cached engine instance before each match_option call. Co-Authored-By: Claude Sonnet 4.6 --- ovos_workshop/skills/ovos.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index e5591811..5de55375 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -2031,6 +2031,7 @@ def ask_selection(self, options: List[str], dialog: str = '', if resp: engine = self._get_selection_engine() + engine.config["min_conf"] = min_conf try: resp = engine.match_option(utterance=resp, options=options, lang=self.lang) except Exception as e: From 388a51cf10d3b05d5dc245378edbf0198a8893a0 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 00:29:49 +0100 Subject: [PATCH 12/22] docs: sync documentation after yesno/selection plugin integration AI-Generated Change: - Model: claude-sonnet-4-6 - Intent: keep docs accurate after feat: yesno/selection agent plugins - Impact: updated README.md, docs/index.md, docs/ovos-skill.md, docs/skill-interaction.md - Changes: - README.md: rewritten with install instructions, config table, and docs links - docs/skill-interaction.md: fixed fallback class name (HeuristicYesNoEngine not YesNoSolver), corrected ask_yesno_plugin default (ovos-solver-yes-no-plugin not None), added line-number citations, corrected failure behaviour table - docs/ovos-skill.md: noted plugin configurability for ask_yesno/ask_selection - docs/index.md: added skill-interaction.md to navigation table - Verified via: manual review against ovos_workshop/skills/ovos.py:1932-2040 Co-Authored-By: Claude Sonnet 4.6 --- README.md | 63 +++++++++++++++++++++++++++++++++++++-- docs/index.md | 1 + docs/ovos-skill.md | 2 ++ docs/skill-interaction.md | 22 +++++++------- 4 files changed, 75 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 960d9c63..d3d02d44 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,62 @@ # OVOS Workshop -OVOS Workshop contains skill base classes and supporting tools to build skills -and applications for OpenVoiceOS systems. + +Base classes, decorators, and helpers for building skills and applications for OpenVoiceOS. + +## Install + +```bash +pip install ovos-workshop +``` + +Runtime dependencies include `ovos-yes-no-plugin` and `ovos-option-matcher-fuzzy-plugin`, which back the `ask_yesno` and `ask_selection` skill methods. + +## Quick Start + +```python +from ovos_workshop.skills.ovos import OVOSSkill +from ovos_workshop.decorators import intent_handler + + +class HelloWorldSkill(OVOSSkill): + + @intent_handler("hello.intent") + def handle_hello(self, message): + self.speak_dialog("hello.response") + + +def create_skill(): + return HelloWorldSkill() +``` + +Register in `pyproject.toml`: + +```toml +[project.entry-points."opm.skills"] +hello-world-skill = "hello_world_skill:HelloWorldSkill" +``` + +## Configuration + +Key settings a skill can accept in its `settings.json`: + +| Key | Default | Description | +|-----|---------|-------------| +| `ask_yesno_plugin` | `ovos-solver-yes-no-plugin` | YesNoEngine plugin used by `ask_yesno()` | +| `ask_selection_plugin` | `ovos-option-matcher-fuzzy-plugin` | OptionMatcherEngine plugin used by `ask_selection()` | + +Both keys can also be set system-wide under the `skills` block in `mycroft.conf`. + +## Documentation + +Full reference is in [`docs/`](docs/index.md): + +- [Skill classes](docs/skill-classes.md) +- [OVOSSkill base class](docs/ovos-skill.md) +- [ask_yesno / ask_selection plugin system](docs/skill-interaction.md) +- [Decorators](docs/decorators.md) +- [Settings](docs/settings.md) +- [Resource files](docs/resource-files.md) + +## License + +Apache 2.0 diff --git a/docs/index.md b/docs/index.md index 90fc7117..c2d97772 100644 --- a/docs/index.md +++ b/docs/index.md @@ -68,6 +68,7 @@ OVOSSkill ovos_workshop/skills/ovos.py | [app.md](app.md) | `OVOSAbstractApplication` | Skill-like app that runs without the intent service | | [game-skill.md](game-skill.md) | `OVOSGameSkill`, `ConversationalGameSkill` | OCP-integrated game loop with converse and auto-save | | [auto-translatable.md](auto-translatable.md) | `UniversalSkill`, `UniversalFallback` | Auto-translate input/output for any language | +| [skill-interaction.md](skill-interaction.md) | `OVOSSkill.ask_yesno`, `OVOSSkill.ask_selection` | Pluggable yes/no and option-selection engines | | [skill-api.md](skill-api.md) | `SkillApi`, `skill_api_method` | Inter-skill RPC over the MessageBus | | [filesystem.md](filesystem.md) | `FileSystemAccess` | Sandboxed, XDG-compliant file storage for skills | | [resource-files.md](resource-files.md) | `SkillResources` | Locale, dialog, vocab, regex, and other resource files | diff --git a/docs/ovos-skill.md b/docs/ovos-skill.md index e2f45eb1..f4b40c5a 100644 --- a/docs/ovos-skill.md +++ b/docs/ovos-skill.md @@ -120,6 +120,8 @@ choice = self.ask_selection(["A", "B", "C"], "Pick one") `get_response` suspends the converse channel for this skill until the user responds or a timeout is hit. Raise `AbortQuestion` to cancel gracefully. +`ask_yesno` and `ask_selection` are backed by pluggable engine plugins. The active plugin can be set per-skill via `settings.json` (`ask_yesno_plugin`, `ask_selection_plugin`) or system-wide in `mycroft.conf` under the `skills` block. Defaults are `ovos-solver-yes-no-plugin` and `ovos-option-matcher-fuzzy-plugin`, both installed as runtime dependencies. See [skill-interaction.md](skill-interaction.md) for full configuration reference. + ## Intent Registration ```python diff --git a/docs/skill-interaction.md b/docs/skill-interaction.md index c15413ea..17ca1ef7 100644 --- a/docs/skill-interaction.md +++ b/docs/skill-interaction.md @@ -77,10 +77,9 @@ else: ### How it works internally 1. `get_response(dialog=prompt, data=data)` — speaks the prompt, records user reply. -2. `_get_yesno_engine()` — loads the configured `YesNoEngine` plugin (if any). -3. If a plugin is loaded: `engine.yes_or_no(question=prompt, response=resp, lang=self.lang)` → `True`, `False`, or `None`. -4. If no plugin: `YesNoSolver().match_yes_or_no(resp, lang=self.lang)` (built-in fallback, always available). -5. `True` → `"yes"`, `False` → `"no"`, `None`/unmatched → raw response. +2. `_get_yesno_engine()` — resolves the plugin name (settings → mycroft.conf → `ovos-solver-yes-no-plugin`), loads it once, and caches it. Falls back to `HeuristicYesNoEngine` if loading fails. `OVOSSkill._get_yesno_engine` — `ovos_workshop/skills/ovos.py:1932` +3. `engine.yes_or_no(question=prompt, response=resp, lang=self.lang)` → `True`, `False`, or `None`. +4. `True` → `"yes"`, `False` → `"no"`, `None`/unmatched → raw response. `OVOSSkill.ask_yesno` — `ovos_workshop/skills/ovos.py:1970` --- @@ -170,8 +169,8 @@ if choice is None: 1. Validates `options` (raises `ValueError` if not a list; returns immediately for 0 or 1 items). 2. Speaks options (as list or numbered menu based on `numeric`). 3. `get_response(dialog=dialog, data=data, num_retries=num_retries)` — speaks optional follow-up dialog, records reply. -4. `_get_selection_engine()` — loads the configured `OptionMatcherEngine` plugin. -5. `engine.match_option(utterance=resp, options=options, lang=self.lang)` — resolves response to a slot. +4. `_get_selection_engine()` — resolves the plugin name (settings → mycroft.conf → `ovos-option-matcher-fuzzy-plugin`), loads it once, and caches it. Falls back to `FuzzyOptionMatcherPlugin` if loading fails. Sets `engine.config["min_conf"] = min_conf` before calling. `OVOSSkill._get_selection_engine` — `ovos_workshop/skills/ovos.py:1951` +5. `engine.match_option(utterance=resp, options=options, lang=self.lang)` — resolves response to a slot. `OVOSSkill.ask_selection` — `ovos_workshop/skills/ovos.py:1990` 6. If the engine raises or returns `None`, `ask_selection` returns `None`. --- @@ -184,7 +183,8 @@ Both methods are backed by pluggable agent engines discovered and loaded via [ov **Plugin type**: `YesNoEngine` (`opm.agents.yesno`) **Config key**: `ask_yesno_plugin` -**Built-in fallback**: `ovos-solver-yes-no-plugin` (always available, no config needed) +**Default plugin**: `ovos-solver-yes-no-plugin` +**Hard fallback**: `HeuristicYesNoEngine` from `ovos-yes-no-plugin` (used when the named plugin fails to load) `YesNoEngine` plugins implement: @@ -235,7 +235,7 @@ def match_option(self, utterance: str, options: List[str], lang: Optional[str] = } ``` -`ask_yesno_plugin` defaults to `None` (built-in `YesNoSolver` used). `ask_selection_plugin` defaults to `ovos-option-matcher-fuzzy-plugin`. +`ask_yesno_plugin` defaults to `ovos-solver-yes-no-plugin`. `ask_selection_plugin` defaults to `ovos-option-matcher-fuzzy-plugin`. Both packages are installed as runtime dependencies of `ovos-workshop`. ### Per-skill override — `settings.json` @@ -341,9 +341,9 @@ Activate system-wide: |----------|-----------------|---------------------| | User says nothing (timeout) | `None` | `None` | | User response unclassifiable | raw transcript string | `None` | -| Plugin fails to load | falls back to `YesNoSolver` | `None` | -| Plugin raises at runtime | falls back to `YesNoSolver` | `None` | -| No plugin configured | `YesNoSolver` used | default fuzzy plugin used | +| Plugin fails to load | falls back to `HeuristicYesNoEngine` | falls back to `FuzzyOptionMatcherPlugin` | +| Plugin raises at runtime | falls back to `HeuristicYesNoEngine` | `None` | +| No plugin configured | `ovos-solver-yes-no-plugin` used | `ovos-option-matcher-fuzzy-plugin` used | `ask_selection` is intentionally strict: any failure returns `None` rather than guessing. Always handle the `None` case in your skill. From e4b27d882f593e7ca77e2833fc140e20b46636b0 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 00:42:14 +0100 Subject: [PATCH 13/22] fix: move ovoscope e2e tests out of build_tests path to prevent hangs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_minicroft blocks up to 60s waiting for READY — too heavy for the standard build_tests matrix. Move test/unittests/test_ask_e2e.py to test/ovoscope/ (not scanned by build_tests) and add a dedicated ovoscope_tests workflow that installs [test] extras and runs that path. Revert install_extras from build_tests.yml. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/build_tests.yml | 1 - .github/workflows/ovoscope_tests.yml | 15 +++++++++++++++ test/ovoscope/__init__.py | 0 test/{unittests => ovoscope}/test_ask_e2e.py | 0 4 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ovoscope_tests.yml create mode 100644 test/ovoscope/__init__.py rename test/{unittests => ovoscope}/test_ask_e2e.py (100%) diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index 237096e4..514024e0 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -12,4 +12,3 @@ jobs: secrets: inherit with: test_path: 'test/unittests' - install_extras: 'test' diff --git a/.github/workflows/ovoscope_tests.yml b/.github/workflows/ovoscope_tests.yml new file mode 100644 index 00000000..eba7c6a6 --- /dev/null +++ b/.github/workflows/ovoscope_tests.yml @@ -0,0 +1,15 @@ +name: Run Ovoscope E2E Tests +on: + push: + branches: [master, dev] + pull_request: + branches: [dev] + workflow_dispatch: + +jobs: + ovoscope_tests: + uses: OpenVoiceOS/gh-automations/.github/workflows/build-tests.yml@dev + secrets: inherit + with: + test_path: 'test/ovoscope' + install_extras: 'test' diff --git a/test/ovoscope/__init__.py b/test/ovoscope/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/unittests/test_ask_e2e.py b/test/ovoscope/test_ask_e2e.py similarity index 100% rename from test/unittests/test_ask_e2e.py rename to test/ovoscope/test_ask_e2e.py From 225c8820779cadce4dd021ae33bf49a0650ae9ee Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 00:53:26 +0100 Subject: [PATCH 14/22] fix: unblock speak(wait=True) in e2e tests by emitting audio_output_end ask_selection speaks each option with wait=True, blocking 15s per speak waiting for recognizer_loop:audio_output_end that never arrives in tests. Emit that event immediately on every speak message so the skill proceeds without delay, then inject the user response once speaking settles. Co-Authored-By: Claude Sonnet 4.6 --- test/ovoscope/test_ask_e2e.py | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/test/ovoscope/test_ask_e2e.py b/test/ovoscope/test_ask_e2e.py index 0d5cbc3a..e23bcf5f 100644 --- a/test/ovoscope/test_ask_e2e.py +++ b/test/ovoscope/test_ask_e2e.py @@ -63,23 +63,33 @@ def handle_selection(self, message: Message): # --------------------------------------------------------------------------- def _inject_response_after_speak(mc, skill_id: str, utterance: str, - session: Session, delay: float = 0.3): - """Wait for the skill to speak (prompt), then inject user utterance. + session: Session, delay: float = 0.1): + """Simulate TTS completion and inject a user response. - Emits directly to ``{skill_id}.converse.get_response`` which is the - internal bus event that ``_wait_response`` reads from. + speak(wait=True) blocks on ``recognizer_loop:audio_output_end``. + We emit that event immediately on every ``speak`` so the skill never + hangs waiting for real TTS. Once the last prompt speak is done we + inject the user's reply via ``{skill_id}.converse.get_response``. """ - spoken = threading.Event() + last_speak_time = [0.0] - def on_speak(msg: str): - spoken.set() + def on_speak(raw: str): + import time + msg = Message.deserialize(raw) + # unblock speak(wait=True) + mc.bus.emit(msg.forward("recognizer_loop:audio_output_end")) + last_speak_time[0] = time.time() - mc.bus.on("speak", on_speak) + mc.bus.on("message", on_speak) def _inject(): - spoken.wait(timeout=10) - mc.bus.remove("speak", on_speak) - import time; time.sleep(delay) + import time + # wait until speaking stops (no new speak for delay seconds) + while True: + time.sleep(0.05) + if last_speak_time[0] and time.time() - last_speak_time[0] >= delay: + break + mc.bus.remove("message", on_speak) mc.bus.emit(Message( f"{skill_id}.converse.get_response", {"utterances": [utterance], "lang": "en-us"}, From e1aed90b1cefc438c1dae0ebb017a77d602dd2f2 Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Thu, 9 Apr 2026 02:21:34 +0100 Subject: [PATCH 15/22] Delete MAINTENANCE_REPORT.md --- MAINTENANCE_REPORT.md | 35 ----------------------------------- 1 file changed, 35 deletions(-) delete mode 100644 MAINTENANCE_REPORT.md diff --git a/MAINTENANCE_REPORT.md b/MAINTENANCE_REPORT.md deleted file mode 100644 index 60f95964..00000000 --- a/MAINTENANCE_REPORT.md +++ /dev/null @@ -1,35 +0,0 @@ -# Maintenance Report — `ovos-workshop` - -## 2026-03-11 — CodeRabbit PR #386 review fixes + CI workflow integration - -**AI Model**: claude-sonnet-4-6 -**Oversight**: Human review pending - -### Actions Taken - -#### Bug Fixes -- `ovos_workshop/resource_files.py:89` — Narrowed `except Exception` to `except ValueError` in `locate_lang_directories`; `tag_distance` only raises `ValueError` for invalid language codes. -- `ovos_workshop/skills/ovos.py:682` — Added error context to initialization failure log (`LOG.exception` with `{e}`). -- `test/unittests/test_abstract_app.py:26` — Fixed constructor typo `__int__` → `__init__` (class constructor was never called). - -#### Test Improvements -- `test/unittests/skills/test_idle_display_skill.py` — Replaced misleading `hasattr(__init__)` check with a proper assertion that `handle_idle` is abstract. -- `test/unittests/skills/test_converse_extended.py` — Tightened `assertIsInstance(dict)` to `assertEqual({})` for `converse_matchers`. -- `test/unittests/skills/test_common_play_extended.py` — Save/restore XDG env vars in `tearDown`; tightened `ocp_cache_dir` assertion with `os.path.basename`; added `initial_count + 1` assertion to `test_register_media_type`. -- `test/unittests/test_decorators_layers_extended.py:73-76` — Added `assertFalse(layers.is_active("nonexistent"))` after `activate_layer` on non-existent layer. -- `test/unittests/skills/test_intent_provider.py:23` — Added actual `DeprecationWarning` assertion using `warnings.catch_warnings`. -- `test/unittests/test_decorators.py` — Removed unnecessary `f`-prefixes from string literals with no interpolation. - -#### Documentation Fixes -- `docs/game-skill.md:11` — Added `text` language specifier to fenced code block. -- `FAQ.md:18,32` — Fixed path examples to use `.` instead of `ovos-workshop/` prefix. - -#### CI Workflow Fixes -- `.github/workflows/license_tests.yml` — Removed invalid empty `with:` block; added `secrets: inherit`. -- `.github/workflows/build_tests.yml` — Added `dev` to push trigger branches. - -#### New CI Workflows Added -- `.github/workflows/test.yml` — Unit tests via shared `build-tests.yml@dev`. -- `.github/workflows/lint.yml` — Ruff linting via shared `lint.yml@dev`. -- `.github/workflows/pip_audit.yml` — Dependency CVE scan via shared `pip-audit.yml@dev`. -- `.github/workflows/repo_health.yml` — Repo hygiene check via shared `repo-health.yml@dev`. From b6d5707c436d0fb662bade561af95f10c6b3622f Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 14:33:26 +0100 Subject: [PATCH 16/22] fix: use bus.on("speak") not on("message") to avoid recursion in e2e tests Listening on "message" caught the audio_output_end we emitted, causing infinite recursion. Listen on "speak" specifically instead. Co-Authored-By: Claude Sonnet 4.6 --- test/ovoscope/test_ask_e2e.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/ovoscope/test_ask_e2e.py b/test/ovoscope/test_ask_e2e.py index e23bcf5f..998bbd44 100644 --- a/test/ovoscope/test_ask_e2e.py +++ b/test/ovoscope/test_ask_e2e.py @@ -73,14 +73,13 @@ def _inject_response_after_speak(mc, skill_id: str, utterance: str, """ last_speak_time = [0.0] - def on_speak(raw: str): + def on_speak(msg: Message): import time - msg = Message.deserialize(raw) - # unblock speak(wait=True) + # unblock speak(wait=True) — emit audio_output_end with same context mc.bus.emit(msg.forward("recognizer_loop:audio_output_end")) last_speak_time[0] = time.time() - mc.bus.on("message", on_speak) + mc.bus.on("speak", on_speak) def _inject(): import time @@ -89,7 +88,7 @@ def _inject(): time.sleep(0.05) if last_speak_time[0] and time.time() - last_speak_time[0] >= delay: break - mc.bus.remove("message", on_speak) + mc.bus.remove("speak", on_speak) mc.bus.emit(Message( f"{skill_id}.converse.get_response", {"utterances": [utterance], "lang": "en-us"}, From 6657d3e01bc0d6576e0e9619e3abfeebef622a3b Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 14:36:18 +0100 Subject: [PATCH 17/22] chore: merge ovoscope e2e tests back into build_tests No need for a separate workflow. Move test_ask_e2e.py back to test/unittests/ and install [test] extras in build_tests so ovoscope and ovos-core are available. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/build_tests.yml | 1 + .github/workflows/ovoscope_tests.yml | 15 --------------- test/ovoscope/__init__.py | 0 test/{ovoscope => unittests}/test_ask_e2e.py | 0 4 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 .github/workflows/ovoscope_tests.yml delete mode 100644 test/ovoscope/__init__.py rename test/{ovoscope => unittests}/test_ask_e2e.py (100%) diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index 514024e0..237096e4 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -12,3 +12,4 @@ jobs: secrets: inherit with: test_path: 'test/unittests' + install_extras: 'test' diff --git a/.github/workflows/ovoscope_tests.yml b/.github/workflows/ovoscope_tests.yml deleted file mode 100644 index eba7c6a6..00000000 --- a/.github/workflows/ovoscope_tests.yml +++ /dev/null @@ -1,15 +0,0 @@ -name: Run Ovoscope E2E Tests -on: - push: - branches: [master, dev] - pull_request: - branches: [dev] - workflow_dispatch: - -jobs: - ovoscope_tests: - uses: OpenVoiceOS/gh-automations/.github/workflows/build-tests.yml@dev - secrets: inherit - with: - test_path: 'test/ovoscope' - install_extras: 'test' diff --git a/test/ovoscope/__init__.py b/test/ovoscope/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/test/ovoscope/test_ask_e2e.py b/test/unittests/test_ask_e2e.py similarity index 100% rename from test/ovoscope/test_ask_e2e.py rename to test/unittests/test_ask_e2e.py From eb213ad6c24d00e6aae14542407f2f9547951467 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 15:12:46 +0100 Subject: [PATCH 18/22] fix: delay audio_output_end emission past sess.is_speaking=True MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FakeBus is synchronous — on_speak fires inline during bus.emit(speak), before the caller sets sess.is_speaking=True. Emitting audio_output_end in the same stack means wait_while_speaking misses it and hangs 15s. Use a daemon thread with a 20ms delay so is_speaking is set first. Co-Authored-By: Claude Sonnet 4.6 --- test/unittests/test_ask_e2e.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py index 998bbd44..92931e76 100644 --- a/test/unittests/test_ask_e2e.py +++ b/test/unittests/test_ask_e2e.py @@ -75,8 +75,13 @@ def _inject_response_after_speak(mc, skill_id: str, utterance: str, def on_speak(msg: Message): import time - # unblock speak(wait=True) — emit audio_output_end with same context - mc.bus.emit(msg.forward("recognizer_loop:audio_output_end")) + # FakeBus is synchronous: audio_output_end must fire AFTER + # sess.is_speaking=True is set (which happens after bus.emit returns). + # A short thread delay ensures the ordering is correct. + def _emit_end(): + time.sleep(0.02) + mc.bus.emit(msg.forward("recognizer_loop:audio_output_end")) + threading.Thread(target=_emit_end, daemon=True).start() last_speak_time[0] = time.time() mc.bus.on("speak", on_speak) From 46b2feccfb3e86bee46bf1ec762bada0d3fdb77a Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 15:30:21 +0100 Subject: [PATCH 19/22] test: remove fragile ovoscope e2e tests for ask_yesno/ask_selection The tests fought TTS wait machinery (speak(wait=True) blocks on recognizer_loop:audio_output_end) without adding coverage beyond what test_skill_interaction.py already provides via mocked get_response. Unit tests are sufficient here. Co-Authored-By: Claude Sonnet 4.6 --- test/unittests/test_ask_e2e.py | 206 --------------------------------- 1 file changed, 206 deletions(-) delete mode 100644 test/unittests/test_ask_e2e.py diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py deleted file mode 100644 index 92931e76..00000000 --- a/test/unittests/test_ask_e2e.py +++ /dev/null @@ -1,206 +0,0 @@ -# Copyright 2026 OpenVoiceOS -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Ovoscope end-to-end tests for ask_yesno and ask_selection.""" -import threading -import unittest - -from ovos_bus_client.message import Message -from ovos_bus_client.session import SessionManager, Session -from ovos_utils.log import LOG -from ovos_workshop.skills.ovos import OVOSSkill - -from ovoscope import get_minicroft, CaptureSession - -# --------------------------------------------------------------------------- -# Shared skill IDs -# --------------------------------------------------------------------------- -YESNO_SKILL_ID = "test.ask.yesno.skill" -SELECT_SKILL_ID = "test.ask.selection.skill" - - -# --------------------------------------------------------------------------- -# Inline test skills -# --------------------------------------------------------------------------- - -class AskYesNoSkill(OVOSSkill): - """Handles 'test.ask.yesno': calls ask_yesno and emits the result.""" - - def initialize(self): - self.add_event("test.ask.yesno", self.handle_yesno) - - def handle_yesno(self, message: Message): - answer = self.ask_yesno("do you want tea") - self.bus.emit(message.forward("test.yesno.result", {"answer": answer})) - self.bus.emit(message.forward("ovos.utterance.handled")) - - -class AskSelectionSkill(OVOSSkill): - """Handles 'test.ask.selection': calls ask_selection and emits the result.""" - - def initialize(self): - self.add_event("test.ask.selection", self.handle_selection) - - def handle_selection(self, message: Message): - options = message.data.get("options", ["alpha", "beta", "gamma"]) - answer = self.ask_selection(options, numeric=True) - self.bus.emit(message.forward("test.selection.result", {"answer": answer})) - self.bus.emit(message.forward("ovos.utterance.handled")) - - -# --------------------------------------------------------------------------- -# Helper — inject a user response after the skill starts listening -# --------------------------------------------------------------------------- - -def _inject_response_after_speak(mc, skill_id: str, utterance: str, - session: Session, delay: float = 0.1): - """Simulate TTS completion and inject a user response. - - speak(wait=True) blocks on ``recognizer_loop:audio_output_end``. - We emit that event immediately on every ``speak`` so the skill never - hangs waiting for real TTS. Once the last prompt speak is done we - inject the user's reply via ``{skill_id}.converse.get_response``. - """ - last_speak_time = [0.0] - - def on_speak(msg: Message): - import time - # FakeBus is synchronous: audio_output_end must fire AFTER - # sess.is_speaking=True is set (which happens after bus.emit returns). - # A short thread delay ensures the ordering is correct. - def _emit_end(): - time.sleep(0.02) - mc.bus.emit(msg.forward("recognizer_loop:audio_output_end")) - threading.Thread(target=_emit_end, daemon=True).start() - last_speak_time[0] = time.time() - - mc.bus.on("speak", on_speak) - - def _inject(): - import time - # wait until speaking stops (no new speak for delay seconds) - while True: - time.sleep(0.05) - if last_speak_time[0] and time.time() - last_speak_time[0] >= delay: - break - mc.bus.remove("speak", on_speak) - mc.bus.emit(Message( - f"{skill_id}.converse.get_response", - {"utterances": [utterance], "lang": "en-us"}, - {"session": session.serialize(), "skill_id": skill_id}, - )) - - t = threading.Thread(target=_inject, daemon=True) - t.start() - return t - - -def _make_trigger(msg_type: str, skill_id: str, - data: dict = None, session_id: str = "e2e-test") -> Message: - sess = Session(session_id) - sess.lang = "en-us" - return Message(msg_type, data or {}, - {"session": sess.serialize(), - "skill_id": skill_id, - "source": "test", "destination": skill_id}) - - -# --------------------------------------------------------------------------- -# Tests: ask_yesno -# --------------------------------------------------------------------------- - -class TestAskYesnoE2E(unittest.TestCase): - - @classmethod - def setUpClass(cls): - LOG.set_level("ERROR") - cls.mc = get_minicroft([YESNO_SKILL_ID], - extra_skills={YESNO_SKILL_ID: AskYesNoSkill}) - - @classmethod - def tearDownClass(cls): - cls.mc.stop() - - def _run(self, user_says: str, session_id: str = "e2e-yesno") -> list: - """Trigger ask_yesno, inject a user reply, return captured messages.""" - trigger = _make_trigger("test.ask.yesno", YESNO_SKILL_ID, - session_id=session_id) - sess = SessionManager.get(trigger) - _inject_response_after_speak(self.mc, YESNO_SKILL_ID, - user_says, sess) - cap = CaptureSession(self.mc) - cap.capture(trigger, timeout=15) - return cap.finish() - - def test_yes_response(self): - msgs = self._run("yes", session_id="e2e-yesno-yes") - results = [m for m in msgs if m.msg_type == "test.yesno.result"] - self.assertTrue(results, "test.yesno.result not emitted") - self.assertEqual(results[0].data["answer"], "yes") - - def test_no_response(self): - msgs = self._run("nope", session_id="e2e-yesno-no") - results = [m for m in msgs if m.msg_type == "test.yesno.result"] - self.assertTrue(results, "test.yesno.result not emitted") - self.assertEqual(results[0].data["answer"], "no") - - def test_unmatched_response_returns_raw(self): - msgs = self._run("maybe later", session_id="e2e-yesno-maybe") - results = [m for m in msgs if m.msg_type == "test.yesno.result"] - self.assertTrue(results, "test.yesno.result not emitted") - self.assertEqual(results[0].data["answer"], "maybe later") - - -# --------------------------------------------------------------------------- -# Tests: ask_selection -# --------------------------------------------------------------------------- - -class TestAskSelectionE2E(unittest.TestCase): - - @classmethod - def setUpClass(cls): - LOG.set_level("ERROR") - cls.mc = get_minicroft([SELECT_SKILL_ID], - extra_skills={SELECT_SKILL_ID: AskSelectionSkill}) - - @classmethod - def tearDownClass(cls): - cls.mc.stop() - - def _run(self, user_says: str, options: list = None, - session_id: str = "e2e-select") -> list: - trigger = _make_trigger("test.ask.selection", SELECT_SKILL_ID, - data={"options": options or ["alpha", "beta", "gamma"]}, - session_id=session_id) - sess = SessionManager.get(trigger) - _inject_response_after_speak(self.mc, SELECT_SKILL_ID, - user_says, sess) - cap = CaptureSession(self.mc) - cap.capture(trigger, timeout=15) - return cap.finish() - - def test_fuzzy_match(self): - msgs = self._run("beta", session_id="e2e-select-beta") - results = [m for m in msgs if m.msg_type == "test.selection.result"] - self.assertTrue(results, "test.selection.result not emitted") - self.assertEqual(results[0].data["answer"], "beta") - - def test_first_option(self): - msgs = self._run("alpha", session_id="e2e-select-alpha") - results = [m for m in msgs if m.msg_type == "test.selection.result"] - self.assertTrue(results, "test.selection.result not emitted") - self.assertEqual(results[0].data["answer"], "alpha") - - -if __name__ == "__main__": - unittest.main() From 4ae7bdadbaf38f8233321a194e55d1cc010b3733 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 15:30:38 +0100 Subject: [PATCH 20/22] chore: drop ovoscope from test deps, revert install_extras No ovoscope e2e tests remain, no need for the heavy dependency. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/build_tests.yml | 1 - pyproject.toml | 9 --------- requirements/test.txt | 1 - 3 files changed, 11 deletions(-) diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index 237096e4..514024e0 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -12,4 +12,3 @@ jobs: secrets: inherit with: test_path: 'test/unittests' - install_extras: 'test' diff --git a/pyproject.toml b/pyproject.toml index 3398479c..16878da9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,15 +23,6 @@ dependencies = [ "padacioso>=1.0.0, <2.0.0", ] -[project.optional-dependencies] -test = [ - "ovos-core>=0.0.8a50", - "ovoscope>=0.1.0", - "pytest", - "pytest-cov", - "ovos-translate-server-plugin", -] - [project.urls] Homepage = "https://github.com/OpenVoiceOS/OVOS-workshop" Repository = "https://github.com/OpenVoiceOS/OVOS-workshop" diff --git a/requirements/test.txt b/requirements/test.txt index 1c04ab60..431c2ec4 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,5 +1,4 @@ ovos-core>=0.0.8a50 -ovoscope>=0.1.0 pytest pytest-cov ovos-translate-server-plugin \ No newline at end of file From 2f223b2c3538bc8cd0347753a3a1e60d4da94f0a Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 15:32:46 +0100 Subject: [PATCH 21/22] test: restore ovoscope e2e tests with proper TTS wait fix Patch SessionManager.wait_while_speaking to a no-op so speak(wait=True) doesn't block on TTS completion. Inject user response via skill.converse.get_response.enable event instead of timing hacks. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/build_tests.yml | 1 + pyproject.toml | 9 ++ requirements/test.txt | 1 + test/unittests/test_ask_e2e.py | 167 ++++++++++++++++++++++++++++++ 4 files changed, 178 insertions(+) create mode 100644 test/unittests/test_ask_e2e.py diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index 514024e0..237096e4 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -12,3 +12,4 @@ jobs: secrets: inherit with: test_path: 'test/unittests' + install_extras: 'test' diff --git a/pyproject.toml b/pyproject.toml index 16878da9..3398479c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,15 @@ dependencies = [ "padacioso>=1.0.0, <2.0.0", ] +[project.optional-dependencies] +test = [ + "ovos-core>=0.0.8a50", + "ovoscope>=0.1.0", + "pytest", + "pytest-cov", + "ovos-translate-server-plugin", +] + [project.urls] Homepage = "https://github.com/OpenVoiceOS/OVOS-workshop" Repository = "https://github.com/OpenVoiceOS/OVOS-workshop" diff --git a/requirements/test.txt b/requirements/test.txt index 431c2ec4..1c04ab60 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,4 +1,5 @@ ovos-core>=0.0.8a50 +ovoscope>=0.1.0 pytest pytest-cov ovos-translate-server-plugin \ No newline at end of file diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py new file mode 100644 index 00000000..6e5cab8c --- /dev/null +++ b/test/unittests/test_ask_e2e.py @@ -0,0 +1,167 @@ +# Copyright 2026 OpenVoiceOS +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Ovoscope end-to-end tests for ask_yesno and ask_selection.""" +import threading +import unittest +from unittest.mock import patch + +from ovos_bus_client.message import Message +from ovos_bus_client.session import SessionManager, Session +from ovos_utils.log import LOG +from ovos_workshop.skills.ovos import OVOSSkill + +from ovoscope import get_minicroft, CaptureSession + +YESNO_SKILL_ID = "test.ask.yesno.skill" +SELECT_SKILL_ID = "test.ask.selection.skill" + + +class AskYesNoSkill(OVOSSkill): + def initialize(self): + self.add_event("test.ask.yesno", self.handle_yesno) + + def handle_yesno(self, message: Message): + answer = self.ask_yesno("do you want tea") + self.bus.emit(message.forward("test.yesno.result", {"answer": answer})) + self.bus.emit(message.forward("ovos.utterance.handled")) + + +class AskSelectionSkill(OVOSSkill): + def initialize(self): + self.add_event("test.ask.selection", self.handle_selection) + + def handle_selection(self, message: Message): + options = message.data.get("options", ["alpha", "beta", "gamma"]) + answer = self.ask_selection(options, numeric=True) + self.bus.emit(message.forward("test.selection.result", {"answer": answer})) + self.bus.emit(message.forward("ovos.utterance.handled")) + + +def _inject_response(mc, skill_id: str, utterance: str, session: Session): + """Inject a user response into the skill's get_response wait loop.""" + ready = threading.Event() + + def on_get_response_enable(msg: Message): + ready.set() + + mc.bus.on("skill.converse.get_response.enable", on_get_response_enable) + + def _inject(): + ready.wait(timeout=10) + mc.bus.remove("skill.converse.get_response.enable", on_get_response_enable) + mc.bus.emit(Message( + f"{skill_id}.converse.get_response", + {"utterances": [utterance], "lang": "en-us"}, + {"session": session.serialize(), "skill_id": skill_id}, + )) + + t = threading.Thread(target=_inject, daemon=True) + t.start() + return t + + +def _make_trigger(msg_type: str, skill_id: str, + data: dict = None, session_id: str = "e2e-test") -> Message: + sess = Session(session_id) + sess.lang = "en-us" + return Message(msg_type, data or {}, + {"session": sess.serialize(), + "skill_id": skill_id, + "source": "test", "destination": skill_id}) + + +class TestAskYesnoE2E(unittest.TestCase): + + @classmethod + def setUpClass(cls): + LOG.set_level("ERROR") + cls.mc = get_minicroft([YESNO_SKILL_ID], + extra_skills={YESNO_SKILL_ID: AskYesNoSkill}) + # speak(wait=True) blocks on TTS completion — patch it out for tests + cls._wait_patch = patch.object(SessionManager, "wait_while_speaking") + cls._wait_patch.start() + + @classmethod + def tearDownClass(cls): + cls._wait_patch.stop() + cls.mc.stop() + + def _run(self, user_says: str, session_id: str = "e2e-yesno") -> list: + trigger = _make_trigger("test.ask.yesno", YESNO_SKILL_ID, session_id=session_id) + sess = SessionManager.get(trigger) + _inject_response(self.mc, YESNO_SKILL_ID, user_says, sess) + cap = CaptureSession(self.mc) + cap.capture(trigger, timeout=15) + return cap.finish() + + def test_yes_response(self): + msgs = self._run("yes", session_id="e2e-yesno-yes") + results = [m for m in msgs if m.msg_type == "test.yesno.result"] + self.assertTrue(results, "test.yesno.result not emitted") + self.assertEqual(results[0].data["answer"], "yes") + + def test_no_response(self): + msgs = self._run("nope", session_id="e2e-yesno-no") + results = [m for m in msgs if m.msg_type == "test.yesno.result"] + self.assertTrue(results, "test.yesno.result not emitted") + self.assertEqual(results[0].data["answer"], "no") + + def test_unmatched_response_returns_raw(self): + msgs = self._run("maybe later", session_id="e2e-yesno-maybe") + results = [m for m in msgs if m.msg_type == "test.yesno.result"] + self.assertTrue(results, "test.yesno.result not emitted") + self.assertEqual(results[0].data["answer"], "maybe later") + + +class TestAskSelectionE2E(unittest.TestCase): + + @classmethod + def setUpClass(cls): + LOG.set_level("ERROR") + cls.mc = get_minicroft([SELECT_SKILL_ID], + extra_skills={SELECT_SKILL_ID: AskSelectionSkill}) + cls._wait_patch = patch.object(SessionManager, "wait_while_speaking") + cls._wait_patch.start() + + @classmethod + def tearDownClass(cls): + cls._wait_patch.stop() + cls.mc.stop() + + def _run(self, user_says: str, options: list = None, + session_id: str = "e2e-select") -> list: + trigger = _make_trigger("test.ask.selection", SELECT_SKILL_ID, + data={"options": options or ["alpha", "beta", "gamma"]}, + session_id=session_id) + sess = SessionManager.get(trigger) + _inject_response(self.mc, SELECT_SKILL_ID, user_says, sess) + cap = CaptureSession(self.mc) + cap.capture(trigger, timeout=15) + return cap.finish() + + def test_fuzzy_match(self): + msgs = self._run("beta", session_id="e2e-select-beta") + results = [m for m in msgs if m.msg_type == "test.selection.result"] + self.assertTrue(results, "test.selection.result not emitted") + self.assertEqual(results[0].data["answer"], "beta") + + def test_first_option(self): + msgs = self._run("alpha", session_id="e2e-select-alpha") + results = [m for m in msgs if m.msg_type == "test.selection.result"] + self.assertTrue(results, "test.selection.result not emitted") + self.assertEqual(results[0].data["answer"], "alpha") + + +if __name__ == "__main__": + unittest.main() From 9d48773f1937e64df9913e51e0ab9bf6361f9ccf Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Apr 2026 15:47:27 +0100 Subject: [PATCH 22/22] fix: restore SessionManager.bus after e2e tests to prevent state leak get_minicroft sets SessionManager.bus globally. Without restoring it, subsequent tests get real wait_while_speaking behaviour instead of the early-return they expect (bus=None). Also scope wait_while_speaking patch to setUp/tearDown so it can't leak if tearDownClass throws. Co-Authored-By: Claude Sonnet 4.6 --- test/unittests/test_ask_e2e.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/test/unittests/test_ask_e2e.py b/test/unittests/test_ask_e2e.py index 6e5cab8c..24b571e9 100644 --- a/test/unittests/test_ask_e2e.py +++ b/test/unittests/test_ask_e2e.py @@ -86,16 +86,21 @@ class TestAskYesnoE2E(unittest.TestCase): @classmethod def setUpClass(cls): LOG.set_level("ERROR") + cls._saved_bus = SessionManager.bus cls.mc = get_minicroft([YESNO_SKILL_ID], extra_skills={YESNO_SKILL_ID: AskYesNoSkill}) - # speak(wait=True) blocks on TTS completion — patch it out for tests - cls._wait_patch = patch.object(SessionManager, "wait_while_speaking") - cls._wait_patch.start() @classmethod def tearDownClass(cls): - cls._wait_patch.stop() cls.mc.stop() + SessionManager.bus = cls._saved_bus + + def setUp(self): + self._wait_patch = patch.object(SessionManager, "wait_while_speaking") + self._wait_patch.start() + + def tearDown(self): + self._wait_patch.stop() def _run(self, user_says: str, session_id: str = "e2e-yesno") -> list: trigger = _make_trigger("test.ask.yesno", YESNO_SKILL_ID, session_id=session_id) @@ -129,15 +134,21 @@ class TestAskSelectionE2E(unittest.TestCase): @classmethod def setUpClass(cls): LOG.set_level("ERROR") + cls._saved_bus = SessionManager.bus cls.mc = get_minicroft([SELECT_SKILL_ID], extra_skills={SELECT_SKILL_ID: AskSelectionSkill}) - cls._wait_patch = patch.object(SessionManager, "wait_while_speaking") - cls._wait_patch.start() @classmethod def tearDownClass(cls): - cls._wait_patch.stop() cls.mc.stop() + SessionManager.bus = cls._saved_bus + + def setUp(self): + self._wait_patch = patch.object(SessionManager, "wait_while_speaking") + self._wait_patch.start() + + def tearDown(self): + self._wait_patch.stop() def _run(self, user_says: str, options: list = None, session_id: str = "e2e-select") -> list: