refactor: route OVOSSkill resource loading through ovos-spec-tools (back-compat)#414
Open
JarbasAl wants to merge 4 commits into
Open
refactor: route OVOSSkill resource loading through ovos-spec-tools (back-compat)#414JarbasAl wants to merge 4 commits into
JarbasAl wants to merge 4 commits into
Conversation
…ack-compat) Part of the OVOS migration onto ovos-spec-tools (OpenVoiceOS/architecture#7). OVOSSkill and the rest of ovos_workshop now route resource access through ovos_spec_tools.LocaleResources directly. The legacy ovos_workshop.resource_files.SkillResources is no longer used **internally** but stays shipped and stays available on every OVOSSkill through a deprecation mixin — every skill in the ecosystem keeps working. ## Back-compat surface preserved All legacy entry points stay on OVOSSkill via the new _LegacyResourcesMixin. Each emits a single DeprecationWarning pointing at the spec-tools-backed replacement and routes through the same cached SkillResources instance so behaviour is unchanged. - self.resources -> real SkillResources (legacy load_*_file roles intact) - self.dialog_renderer -> real MustacheDialogRenderer - self.find_resource -> resource_files.find_resource - self.load_dialog_files -> silent no-op (lazy load via LocaleResources) - self.voc_match_cache -> live _voc_cache getter + warning setter - runtime_requirements / network_requirements classproperties The mixin is a single import away from removal — when the deprecation window closes, dropping _LegacyResourcesMixin from OVOSSkill's bases (and deleting _legacy_resources.py) is the only change needed. Legacy resource roles outside OVOS-INTENT-2 (.list, .value, .word, .template, .qml, skill-side .json) keep loading through the preserved self.resources handle. ## What changed internally (no public-surface impact) - .dialog rendering -> LocaleResources.load_dialog + render (with vocabularies() for <voc> references). Drives render_dialog, speak_dialog, get_response, the skill.error flow. - .voc -> LocaleResources.vocabularies() (eager Adapt-keyword registration) and load_vocabulary (for voc_list / voc_match / remove_voc). - .intent / .entity -> OVOSSkill._locate_lang_file (closest_lang + os.walk); the resolved path is handed to intent_service.register_padatious_intent / register_padatious_entity. - .rx regex files -> OVOSSkill.load_regex_files is kept and self-contained; deprecation independent of (and outliving) the broader resource_files deprecation. - Language directory resolution in OVOSAbstractApplication.get_language_dir delegates to ovos_spec_tools.find_lang_dir (new public helper from ovos-spec-tools 0.6.0a1). - ovos_workshop's internal word_connectors.json / euphony.json read via closest_lang resolution. grep -rn 'resource_files|SkillResources|MustacheDialogRenderer|ResourceFile|RegexExtractor' ovos_workshop/ matches only inside resource_files.py and the legacy mixin. ## One private removal UniversalSkill._load_lang — a dead override constructing a SkillResources; never part of the public API (underscore-prefixed). The base-class load_lang (public, on every OVOSSkill) now returns a LocaleResources instead of a SkillResources, but caller patterns are identical and the load_* methods callers use exist on both. ## Pin bumps - ovos-utils >= 0.11.1a1 — standardize_lang_tag(macro=True) region-preserving fix (without it SessionManager rewrote session.lang from en-US to en). - ovos-spec-tools >= 0.6.0a1 — LocaleResources.find / load_blacklist / load_vocabulary semantics + the empty-msg_type construction allowance + find_lang_dir. - ovos_bus_client >= 2.0.0a4 — Session.deserialize keeps the region. ## Tests Unit-test coverage on the mixin: - test/unittests/skills/test_legacy_resources.py — every shim, every legacy file role, deprecation warnings, source-precedence chain, user-override > skill > core. - test/unittests/skills/test_legacy_skill_omnibus.py — one _LegacyOmnibusSkill that exercises every deprecated API and every legacy resource file type in a single initialize() lifecycle. - test/unittests/skills/test_regex_loading.py — adapt regex via load_regex_files. New ovoscope end-to-end tripwires for the highest-risk paths: - test/end2end/test_regex_skill.py — .rx -> adapt match (pre-existing). - test/end2end/test_universal_skill.py — UniversalSkill cross-language speak_dialog + voc_match. - test/end2end/test_speak_dialog.py — speak_dialog -> render_dialog -> LocaleResources.load_dialog -> render with {slot} + <voc>. - test/end2end/test_padacioso_intent_file.py - register_intent_file -> _locate_lang_file -> padacioso match. - test/end2end/test_padacioso_entity_file.py - register_entity_file -> bus event carrying the resolved .entity path + parsed samples. - test/end2end/test_voc_match.py — voc_match on vanilla OVOSSkill. ## Migration path for skill authors Skills migrate incrementally; each DeprecationWarning points at the spec-tools-backed replacement. No skill *needs* to change in this release cycle. The mixin and resource_files both have a one-major-release deprecation window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 5 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR migrates resource lookup/rendering to ovos-spec-tools LocaleResources, deprecates the legacy ovos_workshop.resource_files APIs (adding runtime deprecation warnings), adds a legacy compatibility mixin for OVOSSkill, updates language handling across skills, and adds extensive unit and end-to-end tests and locale fixtures. ChangesResource handling modernization and deprecation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greetings, human! The automated checks are complete. 👾I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthChecking for any potential repo regressions. 🔄 ✅ All required files present. Latest Version: ✅ ⚖️ License CheckChecking if the licenses are compatible with OVOS. 🧩 ✅ No license violations found (49 packages). License distribution: 12× MIT License, 9× Apache Software License, 7× MIT, 6× Apache-2.0, 2× BSD-3-Clause, 2× ISC License (ISCL), 2× PSF-2.0, 2× Python Software Foundation License, +7 more Full breakdown — 49 packages
Copyright (c) 2022 Phil Ewels Permission is hereby granted, free of charge, to any person obtaining a copy The above copyright notice and this permission notice shall be included in all THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintI've finished my task! Here's the data you need. 📊 ❌ ruff: issues found — see job log 🔒 Security (pip-audit)Scanning for any potential privilege escalations. 🪜 ✅ No known vulnerabilities found (68 packages scanned). 🔨 Build TestsI've finished the digital carpentry on this PR. 🔨 ✅ All versions pass
Keeping the OVOS ecosystem thriving 🌿 |
…cept Four more OVOSSkill methods now emit DeprecationWarning on external call, plus the homescreen feature surface is being dropped entirely. Framework-internal hot-path methods (callers: _startup only): - load_data_files -> _load_data_files - load_vocab_files -> _load_vocab_files Homescreen concept (deprecated entirely, no replacement planned): - register_homescreen_app -> _register_homescreen_app - register_resting_screen -> _register_resting_screen - @homescreen_app decorator - @resting_screen_handler decorator The four method renames keep public-name shims that delegate to the private implementation; framework callsites in _startup / _register_app_launcher now use the private names directly. Subclass overrides of the public name are no longer invoked by the framework. Also drops the _caller_is_internal stack-walking suppression in resource_files.py: deprecation warnings are valuable signal and must not be hidden from workshop's own logs. Audit confirms no workshop internals call the deprecated public surface except the legacy mixin itself (downstream of explicit user opt-in via self.resources etc.). Tests: test_legacy_skill_omnibus extended to exercise the four new shims and assert their DeprecationWarnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ovos_workshop/skills/ovos.py (1)
739-763:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't mask the original startup failure when
statusis still unset.If
_init_settings()orbind()throws beforeself.statusis assigned, theexceptpath raises a secondAttributeErrorhere and hides the real cause. That's exactly what the current CI failures are showing.💡 Suggested fix
except Exception as e: - self.status.set_error(str(e)) + if hasattr(self, "status"): + self.status.set_error(str(e)) + else: + LOG.exception("Skill startup failed before ProcessStatus initialization") # If an exception occurs, attempt to clean up the skill try: self.default_shutdown()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ovos_workshop/skills/ovos.py` around lines 739 - 763, The except block can raise AttributeError when self.status was never set (e.g., if _init_settings() or bind() failed); update the exception handler to avoid masking the original error by guarding access to self.status and default_shutdown: check hasattr(self, "status") and self.status is not None before calling self.status.set_error(str(e)) or self.status.* methods, and protect default_shutdown call similarly (or call it only if it exists), then re-raise the original exception; reference symbols: self.status, _init_settings, bind, default_shutdown, self.status.set_error.
🧹 Nitpick comments (1)
ovos_workshop/skills/util.py (1)
30-50: ⚡ Quick winCache locale JSON resolution to avoid repeated filesystem I/O.
_load_workshop_jsoncurrently rescans locale dirs and re-reads JSON on each call;join_word_listcan hit this path frequently.💡 Suggested refactor
+from functools import lru_cache import json import os from os.path import dirname, isdir, isfile, join @@ +@lru_cache(maxsize=1) +def _available_locale_dirs() -> Dict[str, str]: + return {lang: str(path) for lang, path in iter_locale_dirs(_LOCALE_ROOT)} + +@lru_cache(maxsize=64) def _load_workshop_json(name: str, lang: str) -> Optional[Dict]: @@ - available = dict(iter_locale_dirs(_LOCALE_ROOT)) # {lang_norm: Path} + available = _available_locale_dirs() # {lang_norm: path} @@ - path = join(str(available[best]), f"{name}.json") + path = join(available[best], f"{name}.json")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ovos_workshop/skills/util.py` around lines 30 - 50, The function _load_workshop_json repeatedly rescans locale dirs and reloads JSON on every call (used frequently by join_word_list); cache results to avoid filesystem I/O by memoizing the available locale mapping and parsed JSON per (name, lang) pair. Specifically, keep a module-level cache (or use functools.lru_cache) for iter_locale_dirs(_LOCALE_ROOT) output (used in available = dict(iter_locale_dirs(...))) and for loaded JSON results of json.load(path) keyed by (name, best) so subsequent calls return the cached dict instead of re-reading the file; ensure cache is invalidated/refreshable if needed (e.g., provide a clear/reset helper) and preserve current behavior when files are missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ovos_workshop/app.py`:
- Line 4: The module removed the join import but settings_path still uses join,
causing runtime/lint failures; re-add join to the import list so settings_path
can use it—specifically update the top-level import that currently reads "from
ovos_spec_tools import find_lang_dir, standardize_lang" to also import join so
the function/settings_path and any uses of join resolve correctly.
In `@ovos_workshop/skills/_legacy_resources.py`:
- Around line 59-69: The _legacy_skill_resources method currently snapshots
self.lang into SkillResources and never updates; change it to use
self._resource_lang when constructing SkillResources and add logic to
invalidate/rebuild the cached self._skill_resources_compat whenever
self._resource_lang changes (store the last used lang or compare current
self._resource_lang to the lang on the cached SkillResources), so callers like
resources, dialog_renderer and find_resource get a SkillResources instance
matching the current _resource_lang; update the same pattern in the other
shim(s) referenced (lines 127-147) to ensure cache is refreshed on language
change.
In `@ovos_workshop/skills/ovos.py`:
- Around line 744-748: Startup now calls private implementations
(_load_data_files, _register_skill_json, _register_decorated,
_register_app_launcher, _register_resting_screen) which bypasses subclass
overrides; change calls so public methods (load_data_files,
register_skill_json/register_decorated if present, register_homescreen_app,
register_resting_screen) are invoked first and fall back to their underscored
counterparts if the public method is not overridden. Update _startup,
_register_app_launcher, and any other locations (e.g., around lines 791-793 and
2403-2439) to call the public shim methods like load_data_files and
register_homescreen_app instead of directly calling
_load_data_files/_register_app_launcher, preserving subclass override
compatibility while keeping private implementations as fallbacks.
In `@ovos_workshop/skills/util.py`:
- Around line 49-50: The locale JSON loader opens files using the system default
encoding; change the open call in ovos_workshop/skills/util.py to explicitly use
UTF-8 by passing encoding='utf-8' (e.g., in the with open(path, ...) statement
that wraps json.load), so that parsing localized content works correctly across
environments.
In `@test/end2end/test_regex_skill.py`:
- Around line 72-75: The on_speak handler currently sets speak_event for every
speak message which can unblock the test prematurely; change on_speak (and the
test wiring around seen_speaks/speak_event) to only set speak_event when the
utterance matches the expected output (e.g., compare
msg.data.get("utterance","") against the expected string/regex or check
substring) and still append seen_speaks as needed, so the wait only wakes for
the regex handler's emission.
In `@test/unittests/skills/test_legacy_skill_omnibus.py`:
- Around line 184-185: The tearDownClass currently swallows all exceptions from
cls.skill.default_shutdown() with "except Exception: pass"; change this to catch
Exception as e and handle it instead of silencing—either re-raise after logging
or call a test logger/assertFail to surface the error; specifically update the
tearDownClass method to wrap cls.skill.default_shutdown() in try/except
Exception as e and call something like logging.exception or raise to ensure
failures are reported rather than ignored.
---
Outside diff comments:
In `@ovos_workshop/skills/ovos.py`:
- Around line 739-763: The except block can raise AttributeError when
self.status was never set (e.g., if _init_settings() or bind() failed); update
the exception handler to avoid masking the original error by guarding access to
self.status and default_shutdown: check hasattr(self, "status") and self.status
is not None before calling self.status.set_error(str(e)) or self.status.*
methods, and protect default_shutdown call similarly (or call it only if it
exists), then re-raise the original exception; reference symbols: self.status,
_init_settings, bind, default_shutdown, self.status.set_error.
---
Nitpick comments:
In `@ovos_workshop/skills/util.py`:
- Around line 30-50: The function _load_workshop_json repeatedly rescans locale
dirs and reloads JSON on every call (used frequently by join_word_list); cache
results to avoid filesystem I/O by memoizing the available locale mapping and
parsed JSON per (name, lang) pair. Specifically, keep a module-level cache (or
use functools.lru_cache) for iter_locale_dirs(_LOCALE_ROOT) output (used in
available = dict(iter_locale_dirs(...))) and for loaded JSON results of
json.load(path) keyed by (name, best) so subsequent calls return the cached dict
instead of re-reading the file; ensure cache is invalidated/refreshable if
needed (e.g., provide a clear/reset helper) and preserve current behavior when
files are missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbb8676b-dfdc-4473-8eae-926b41640555
📒 Files selected for processing (32)
.gitignoreovos_workshop/app.pyovos_workshop/decorators/__init__.pyovos_workshop/resource_files.pyovos_workshop/skills/_legacy_resources.pyovos_workshop/skills/auto_translatable.pyovos_workshop/skills/converse.pyovos_workshop/skills/ovos.pyovos_workshop/skills/util.pypyproject.tomltest/end2end/__init__.pytest/end2end/locale/en-US/drink.entitytest/end2end/locale/en-US/greet.dialogtest/end2end/locale/en-US/play.rxtest/end2end/locale/en-US/salutation.voctest/end2end/locale/en-US/wave.intenttest/end2end/locale/en-US/yes.voctest/end2end/test_padacioso_entity_file.pytest/end2end/test_padacioso_intent_file.pytest/end2end/test_regex_skill.pytest/end2end/test_speak_dialog.pytest/end2end/test_universal_skill.pytest/end2end/test_voc_match.pytest/end2end/universal_locale/locale/en-US/affirmative.voctest/end2end/universal_locale/locale/en-US/echo.dialogtest/end2end/universal_locale/locale/en-US/fact.intenttest/end2end/universal_locale/locale/pt-PT/fact.intenttest/unittests/skills/test_base.pytest/unittests/skills/test_legacy_resources.pytest/unittests/skills/test_legacy_skill_omnibus.pytest/unittests/skills/test_ovos.pytest/unittests/skills/test_regex_loading.py
Comment on lines
+72
to
+75
| def on_speak(msg): | ||
| seen_speaks.append(msg.data.get("utterance", "")) | ||
| speak_event.set() | ||
|
|
Contributor
There was a problem hiding this comment.
Gate the wait event on the expected utterance to prevent flakiness.
on_speak sets the event for every speak message. If unrelated speech arrives first, the wait can unblock before the regex handler emits its output.
Suggested fix
def on_speak(msg):
- seen_speaks.append(msg.data.get("utterance", ""))
- speak_event.set()
+ utt = msg.data.get("utterance", "")
+ seen_speaks.append(utt)
+ if "playing music" in utt:
+ speak_event.set()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/end2end/test_regex_skill.py` around lines 72 - 75, The on_speak handler
currently sets speak_event for every speak message which can unblock the test
prematurely; change on_speak (and the test wiring around
seen_speaks/speak_event) to only set speak_event when the utterance matches the
expected output (e.g., compare msg.data.get("utterance","") against the expected
string/regex or check substring) and still append seen_speaks as needed, so the
wait only wakes for the regex handler's emission.
Comment on lines
+184
to
+185
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether silent broad-exception swallowing exists in this test module
rg -nP 'except\s+Exception:\s*\n\s*pass' test/unittests/skills/test_legacy_skill_omnibus.pyRepository: OpenVoiceOS/ovos-workshop
Length of output: 51
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="test/unittests/skills/test_legacy_skill_omnibus.py"
# Print the claimed area with line numbers
nl -ba "$file" | sed -n '165,205p'Repository: OpenVoiceOS/ovos-workshop
Length of output: 112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="test/unittests/skills/test_legacy_skill_omnibus.py"
awk 'NR>=165 && NR<=205 { printf "%d:%s\n", NR, $0 }' "$file"Repository: OpenVoiceOS/ovos-workshop
Length of output: 2046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="test/unittests/skills/test_legacy_skill_omnibus.py"
# Show the import section and any warnings usage
rg -n '^\s*(import|from)\s+warnings|warnings\.' "$file" || true
head -n 60 "$file"Repository: OpenVoiceOS/ovos-workshop
Length of output: 2616
Avoid silently swallowing teardown exceptions in tearDownClass.
In test/unittests/skills/test_legacy_skill_omnibus.py lines 184–185, except Exception: pass hides failures from cls.skill.default_shutdown().
Proposed fix
- except Exception:
- pass
+ except Exception as e:
+ warnings.warn(
+ f"default_shutdown failed during teardown: {e!r}",
+ RuntimeWarning
+ )🧰 Tools
🪛 Ruff (0.15.14)
[error] 184-185: try-except-pass detected, consider logging the exception
(S110)
[warning] 184-184: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/unittests/skills/test_legacy_skill_omnibus.py` around lines 184 - 185,
The tearDownClass currently swallows all exceptions from
cls.skill.default_shutdown() with "except Exception: pass"; change this to catch
Exception as e and handle it instead of silencing—either re-raise after logging
or call a test logger/assertFail to surface the error; specifically update the
tearDownClass method to wrap cls.skill.default_shutdown() in try/except
Exception as e and call something like logging.exception or raise to ensure
failures are reported rather than ignored.
- app.py: restore missing `from os.path import join` (CI red). - _LegacyResourcesMixin: key the SkillResources cache on (res_dir, _resource_lang) so UniversalSkill (resources in internal_language) and plain OVOSSkill instances with session-language changes both see the right files. Previously snapshot self.lang on first access, breaking multilingual back-compat. - skills/ovos.py: revert load_data_files / load_vocab_files / register_homescreen_app / register_resting_screen renames to preserve subclass overrides. _startup keeps calling the public names so legacy override patterns continue to participate in startup. Homescreen pair retains in-body deprecation warnings (homescreen concept is being removed entirely). - skills/util.py: open localized JSON with explicit utf-8 encoding. - test_legacy_skill_omnibus: drop the four-shim assertions, keep the homescreen-concept deprecation assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every deprecated surface now emits both a DeprecationWarning (visible to IDEs, -W error::DeprecationWarning, runtime warning filters) AND a LOG.warning line so consumers tailing logs cannot miss them. _legacy_resources.py - voc_match_cache setter: add @deprecated log alongside existing warnings.warn. - network_requirements: add warnings.warn alongside existing LOG.warning. - new public-name shims: register_homescreen_app and register_resting_screen — both signals + @deprecated decorator, delegate to the _-prefixed implementation in OVOSSkill. skills/ovos.py - load_regex_files: add LOG.warning alongside the existing warnings.warn (gated on a .rx file being present so silent skills stay silent). - register_homescreen_app -> _register_homescreen_app: framework calls the private name; emits warn + LOG only on the gated @homescreen_app path so non-users stay silent. - register_resting_screen -> _register_resting_screen: same pattern, only emits when a @resting_screen_handler-tagged method is found. - _startup / _register_app_launcher updated to call the private names. Subclass overrides of register_homescreen_app / register_resting_screen no longer participate in startup — acceptable because the whole concept is being removed. decorators/__init__.py - @homescreen_app and @resting_screen_handler now log + warn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #413 (closed during squash). Part of the OVOS migration onto ovos-spec-tools (architecture#7).
OVOSSkilland the rest ofovos_workshopnow route resource access throughovos_spec_tools.LocaleResourcesdirectly. The legacyovos_workshop.resource_files.SkillResourcesis no longer used internally but stays shipped and stays available on everyOVOSSkillthrough a deprecation mixin — every skill in the ecosystem keeps working.Back-compat surface preserved
All legacy entry points stay on
OVOSSkillvia the new_LegacyResourcesMixin. Each emits a singleDeprecationWarningand routes through the same cachedSkillResourcesinstance so behaviour is unchanged.self.resourcesSkillResources.self.resources.load_dialog/render_dialog/load_vocab/load_list_file/.value/.template/.word/.jsonall keep working.speak_dialog,voc_match, …) orovos_spec_tools.LocaleResourcesdirectly.self.dialog_rendererMustacheDialogRenderer.render(name, data)works unchanged.OVOSSkill.render_dialog(delegates toovos_spec_tools.render).self.find_resource(name, dirname=None, lang=None)ovos_workshop.resource_files.find_resource.ovos_spec_tools.LocaleResources.find.self.load_dialog_files(root=None)LocaleResourcesis lazy).self.voc_match_cache_voc_cachegetter; setter emits warning + accepts dict.voc_match/voc_listcache internally.runtime_requirements/network_requirementsclasspropertyshims; subclass overrides honoured.The mixin is a single
importaway from removal — when the deprecation window closes, dropping_LegacyResourcesMixinfromOVOSSkill's bases (and deleting_legacy_resources.py) is the only change needed.Other resource roles outside OVOS-INTENT-2 (
.list,.value,.word,.template,.qml, skill-side.json) keep loading through the preservedself.resourceshandle.What changed internally (no public-surface impact)
.dialog->LocaleResources.load_dialog+ovos_spec_tools.renderwithvocabularies()for<voc>references. Drivesrender_dialog,speak_dialog,get_response, theskill.errorflow..voc->LocaleResources.vocabularies()(eager Adapt-keyword registration) andload_vocabulary(forvoc_list/voc_match/remove_voc)..intent/.entity->OVOSSkill._locate_lang_file(closest_lang + os.walk); resolved path handed tointent_service.register_padatious_{intent,entity}..rxregex files ->OVOSSkill.load_regex_filesis kept and self-contained; deprecation independent of (and outliving) the broaderresource_filesdeprecation.OVOSAbstractApplication.get_language_dirdelegates toovos_spec_tools.find_lang_dir(new public helper in ovos-spec-tools 0.6.0a1).word_connectors.json/euphony.jsonread viaclosest_langresolution.grep -rn 'resource_files|SkillResources|MustacheDialogRenderer|ResourceFile|RegexExtractor' ovos_workshop/matches only insideresource_files.pyand the legacy mixin.One private removal
UniversalSkill._load_lang— a dead override constructing aSkillResources; never part of the public API (underscore-prefixed). The base-classload_lang(public, on everyOVOSSkill) now returns aLocaleResourcesinstead of aSkillResources, but caller patterns are identical and theload_*methods callers use exist on both.Pin bumps
ovos-utils>=0.11.1a1standardize_lang_tag(macro=True)region-preserving fix (ovos-utils#377). Without itSessionManagerrewrotesession.langfromen-UStoenon every message.ovos-spec-tools>=0.6.0a1LocaleResources.find/load_blacklist/load_vocabulary+ empty-msg_type construction +find_lang_dir.ovos_bus_client>=2.0.0a4Session.deserializekeeps the region.Tests
Unit-test coverage on the mixin:
test/unittests/skills/test_legacy_resources.py— every shim, every legacy file role, deprecation warnings, source-precedence chain, user-override > skill > core.test/unittests/skills/test_legacy_skill_omnibus.py— one_LegacyOmnibusSkillexercising every deprecated API and every legacy resource file type in a singleinitialize()lifecycle. Single-file tripwire.test/unittests/skills/test_regex_loading.py— Adapt regex viaload_regex_files.New ovoscope end-to-end tripwires for the highest-risk paths:
test/end2end/test_regex_skill.py— .rx → Adapt match.test/end2end/test_universal_skill.py—UniversalSkillcross-languagespeak_dialog+voc_match+ realistic Portuguese-user-with-English-skill round trip.test/end2end/test_speak_dialog.py—speak_dialog→render_dialog→LocaleResources.load_dialog→renderwith{slot}+<voc>.test/end2end/test_padacioso_intent_file.py—register_intent_file→_locate_lang_file→ padacioso match.test/end2end/test_padacioso_entity_file.py—register_entity_file→ bus event carrying the resolved .entity path + parsed samples. (Asserts on the registration plumbing, not slot-fill —.entityfiles in OVOS are training-data hints, not strict constraints.)test/end2end/test_voc_match.py—voc_matchon vanillaOVOSSkill.Migration path for skill authors
Skills migrate incrementally; each
DeprecationWarningpoints at the spec-tools-backed replacement. No skill needs to change in this release cycle. The mixin andresource_filesboth have a one-major-release deprecation window.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Deprecations
Tests
Chores