Commit 6d87f1d
chore: docs tests and misc optimizations (#752)
* Add required project configuration and documentation files
- pyproject.toml: Python project configuration (required for build)
- AUDIT.md: Known issues and technical debt documentation
- QUICK_FACTS.md: Machine-readable project reference
- docs/: Architecture and feature documentation
- .env: Environment configuration
These files were merged but not committed. Required for CI builds to succeed.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* update
* Performance optimizations: fix race conditions, reduce per-utterance overhead
**Priority 1 — Race Conditions (correctness + perf)**
- Add lock to _unload_plugin_skill to prevent concurrent dict mutation (skill_manager.py:585)
- Snapshot plugin_skills dict inside lock for safe iteration in 4 methods:
send_skill_list, deactivate_skill, activate_skill, deactivate_except
Prevents RuntimeError: dictionary changed size during iteration
- Replace busy-wait in _collect_fallback_skills with threading.Event signaling
(fallback_service.py:122-125) — reduces CPU usage on utterances reaching fallback
**Priority 2 — Per-Utterance Work (latency)**
- Replace threading.Event().wait(1) with self._stop_event.wait(1)
(skill_manager.py:462) — reuses event, correctly respects stop signal
- Move migration_map dict and re.compile regex to module-level constants
(service.py) — 15 utterances/second → rebuilding these on every pipeline stage
- Guard create_daemon calls with config check before spawning thread
(service.py:322, 352) — skip thread creation when open_data.intent_urls not configured
**Priority 3 — Minor Overhead**
- Change _logged_skill_warnings from list to set (O(1) lookup vs O(n))
(skill_manager.py:111)
- Cache sorted plugins in all 3 transformer services (transformers.py)
Invalidate cache on load_plugins()
- Read blacklist once before plugin scan loop instead of per-skill
(skill_manager.py:361)
All 65 unit tests pass. Coverage maintained at 60% for ovos_core.skill_manager.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Update docs: performance optimizations, race condition fixes, audit report
- FAQ.md: Add comprehensive Performance section documenting all optimizations (thread-safe loading, event signaling, caching, etc.)
- MAINTENANCE_REPORT.md: Add detailed entry for 2026-03-11 performance optimization work
- AUDIT.md: Document fixed race conditions (plugin_skills dict, busy-wait, temporary events)
- SUGGESTIONS.md: Add S-007 marking all performance improvements as ADDRESSED
All changes cross-referenced with code locations and commit SHA.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Fix documented TODOs: skill uninstall + minor clarifications
**S-002: Implement skill uninstall (VALID FIX)**
- Implement handle_uninstall_skill() to call pip_uninstall() for skill packages
- Replace 'not implemented' error with actual uninstall logic
- Convert skill_id to package name (dots → hyphens)
- Validate skill parameter before attempting uninstall
**Minor TODO clarifications**
- Docker detection warning in launch_standalone() for container environments
- Clarified voc_match() TODO: explain why StopService reimplements instead of using ovos_workshop
(StopService is not a skill; voc_match is service-specific)
**Test updates**
- Updated test_handle_uninstall_skill to expect 'no packages to install' instead of 'not implemented'
**S-006 (DEFERRED)**
- Reverted external skills registry implementation
- These TODOs are architectural limitations, not missing features
- External skills run in separate processes and only communicate via messagebus
- They cannot be listed, activated, or deactivated by ovos-core
All 65 unit tests pass.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Update docs: clarify S-002 implementation and S-006 architectural limitation
- SUGGESTIONS.md: Mark S-002 as ADDRESSED with implementation details
- SUGGESTIONS.md: Document S-006 as architectural limitation (external skills run in separate processes)
- SUGGESTIONS.md: Explain correct pattern for external skills (self-advertise + respond to bus messages)
- MAINTENANCE_REPORT.md: Add entry for S-002 implementation with rationale on S-006
- All references include commit SHAs and line numbers
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* update
* fix: bus listener leaks, None log crash, and intent service startup timeout
- converse_service: wrap bus.on/event.wait/bus.remove in try/finally so
the skill.converse.pong listener is always removed even if handle_ack
raises; add skill_id guard against malformed pong; change can_handle
default True→False (non-responding skill should not converse)
- stop_service: same try/finally + skill_id guard + can_handle default
True→False for skill.stop.pong listener
- service.py: fix LOG.info string concat crash when cancel_word is None
(use f-string instead of + operator)
- skill_manager: add configurable max_wait to wait_for_intent_service
(default 300 s via skills.intent_service_timeout); raises descriptive
RuntimeError with instructions instead of looping forever
Note: sound config caching was not applied — Configuration() is a live
object in OVOS that reflects runtime changes without restart.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor: StopService inherits OVOSAbstractApplication for voc_match
StopService now follows the same pattern as CommonQAService and
OCPPipelineMatcher: double-inheriting ConfidenceMatcherPipeline and
OVOSAbstractApplication so that vocabulary loading and voc_match/voc_list
are provided by the shared ovos-workshop infrastructure instead of a
hand-rolled reimplementation.
Changes:
- Add OVOSAbstractApplication to base classes; call both __init__s with
skill_id="stop.openvoiceos" and resources_dir=dirname(__file__)
- Remove load_resource_files(), _voc_cache dict, _get_closest_lang(),
and the custom voc_match() override (~60 lines deleted)
- Replace self._voc_cache[lang]['stop'] in match_low with self.voc_list()
- Replace _get_closest_lang() guards in match_* with voc_list() emptiness
check (voc_list returns [] for unknown langs — no crash, no None sentinel)
- Rename all locale/*.intent files to *.voc so OVOSSkill resource loading
finds them via the standard ResourceType("vocab", ".voc", ...) path
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add unit tests for StopService — 96% coverage
27 tests covering:
- _collect_stop_skills: no active skills, can_handle True/False, timeout
cleanup (try/finally), exception cleanup, malformed pong guard (no
skill_id), blacklisted skills excluded from ping
- handle_stop_confirmation: error branch, response-mode abort_question,
converse force_timeout, TTS stop when speaking, skill_id fallback from
msg_type
- match_high: no vocab → None, stop+no active skills → global stop,
stop+active skills → skill stop, global_stop voc → global stop
- match_medium: no voc → None, stop/global_stop voc delegates to match_low
- match_low: empty voc_list → None, below threshold → None, active skill
confidence boost, above threshold → skill stop
- handle_global_stop / handle_skill_stop bus message forwarding
- get_active_skills session delegation
- shutdown removes both bus listeners
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(e2e): add end2end tests for StopService OVOSAbstractApplication refactor
Fix existing stop e2e tests:
- Add 'stop.openvoiceos.stop.response' to all ignore_messages lists in
test_stop.py — StopService now subclasses OVOSAbstractApplication so it
responds to the mycroft.stop broadcast like other pipeline-plugin skills
(common_query, ocp, persona already filtered there)
New test file test_stop_refactor.py — 5 tests across 4 classes:
TestGlobalStopVocabulary (no skills loaded):
- test_global_stop_voc_no_active_skills: 'stop everything' matches
global_stop.voc and emits stop:global (regression: .voc rename works)
- test_stop_voc_exact_still_works: bare 'stop' still matches stop.voc
(regression: .voc rename did not break the stop vocabulary)
TestGlobalStopVocWithActiveSkill (count skill loaded):
- test_global_stop_voc_with_active_skill: 'stop everything now' emits
stop:global even when a skill is in the active list — verifying that
global_stop.voc takes priority over the stop:skill path
TestStopSkillCanHandleFalse (count skill loaded):
- test_stop_with_active_skill_ping_pong: full stop ping-pong sequence
with a running skill — verifies stop.ping → skill.stop.pong(can_handle=True)
→ stop:skill → {skill}.stop → {skill}.stop.response chain
TestStopServiceAsSkill (no skills loaded):
- test_stop_service_emits_activate_and_stop_response: explicitly asserts
that stop.openvoiceos.activate and stop.openvoiceos.stop.response appear
in the message sequence, confirming StopService participates in the
OVOSSkill stop lifecycle
Also installed missing test dependencies:
ovos-skill-count, ovos-skill-parrot, ovos-skill-hello-world,
ovos-skill-fallback-unknown, ovos-padatious-pipeline-plugin (from local
workspace), ovos-adapt-pipeline-plugin (from local workspace),
ovos-utterance-plugin-cancel (from local workspace)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(S-003): strengthen validate_skill with GitHub API validation
- Parse owner/repo from URL; call api.github.com/repos/{owner}/{repo}/contents/
- Reject repos that don't exist (HTTP 404)
- Reject bare setup.py-only repos (legacy Mycroft packaging)
- Fetch pyproject.toml/setup.cfg and reject if MycroftSkill or CommonPlaySkill found
- Fail-open on network errors and unexpected API status codes (3 s timeout)
- Fix 3 existing tests that assumed no network call (now mock requests.get/validate_skill)
- Add 10 new unit tests covering all validation branches
- Add test_converse_service.py (43 tests, 81% coverage)
- Update FAQ.md and MAINTENANCE_REPORT.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add unit tests for fallback_service, transformers, and intent service
- test_fallback_service.py: 34 tests, 93% coverage
- handle_register/deregister_fallback, _fallback_allowed (ACCEPT_ALL/BLACKLIST/WHITELIST),
_collect_fallback_skills (ping-pong, timeouts, blacklisted sessions),
_fallback_range, match_high/medium/low delegation, shutdown
- test_transformers.py: 40 tests, 66% coverage
- All three transformer services (Utterance/Metadata/Intent)
- Plugin loading, priority ordering + caching, transform chaining,
exception swallowing, context merging, session key stripping
- test_intent_service_extended.py: 37 tests, raises service.py from 0% to 49%
- _handle_transformers, disambiguate_lang, get_pipeline_matcher (migration map),
get_pipeline, context handlers, send_cancel_event/send_complete_intent_failure,
_emit_match_message, handle_utterance (cancel/no-match), handle_get_intent, shutdown
Total: 247 unit tests passing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(e2e): add intent pipeline routing end-to-end tests
4 tests covering basic pipeline routing with ovos-skill-count:
- Padatious intent matched end-to-end (full handler lifecycle)
- High-priority pipeline stage handles before lower-priority stages
- Unrecognized utterance produces complete_intent_failure + error sound
- Blacklisted skill falls through to complete_intent_failure
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(locale): sync .voc files from translations/ (source of truth)
Regenerate all stop.voc and global_stop.voc files directly from
translations/{lang}/intents.json to ensure they stay in sync.
Changes:
- Preserve phrase order from translations (previously sorted alphabetically)
- Add missing phrases that existed in translations but not locale
- Remove phrases in locale that were not in translations
- Normalize fa-IR -> fa-ir (locale dir is always lowercase)
- nl-NL and nl-nl both exist in translations; nl-nl (canonical) wins
- nl-be has no global_stop translation — global_stop.voc intentionally absent
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix pyproject.toml
* chore: Add ovoscope end-to-end tests with bus coverage report to CI
- Created .github/workflows/ovoscope.yml using gh-automations@dev reusable workflow
- Enables bus coverage tracking for behavioural test metrics
- Posts 🔌 Skill Tests (ovoscope) and 🚌 Bus Coverage sections to PR comments
- Requires Adapt and Padatious pipelines for comprehensive intent testing
- Updated FAQ.md with CI/Testing section explaining bus coverage
- Updated QUICK_FACTS.md with testing workflow reference
- Updated MAINTENANCE_REPORT.md with session log
Bus coverage complements code coverage by showing which bus message types
are exercised during tests, helping identify gaps in skill interaction testing.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix pyproject.toml
* fix
* Delete .coverage
* .
* .
* .
* fix: thread names in bus coverage report
* more workflows
* coderabbit
---------
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>1 parent 8ea3f6e commit 6d87f1d
File tree
61 files changed
+4804
-716
lines changed- .github/workflows
- ovos_core
- intent_services
- locale
- ca-es
- da-dk
- de-de
- en-us
- es-es
- eu
- fa-ir
- fr-fr
- gl-es
- it-it
- nl-be
- nl-nl
- pl-pl
- pt-br
- pt-pt
- uk-ua
- test
- end2end
- unittests
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
61 files changed
+4804
-716
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
| 17 | + | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
25 | 30 | | |
26 | 31 | | |
27 | 32 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
4 | 68 | | |
5 | 69 | | |
6 | 70 | | |
| |||
100 | 164 | | |
101 | 165 | | |
102 | 166 | | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
0 commit comments