Conversation
voc_list used its lang argument only to build the cache key and then read
the vocabulary from self.resources, which is bound to self.lang. Asking for
another language returned the skill's current-language words: with the skill
in en-US, voc_list("colour", "es-ES") returned the en-US list, so
voc_match("quiero rojo", "colour", lang="es-ES") was False while
voc_match("I want red", "colour", lang="es-ES") was True.
voc_list now loads the SkillResources for the requested lang through
load_lang, which already caches one SkillResources per language; the
per-language cache key is unchanged.
Regression tests: test_voc_list and test_voc_match in
test/unittests/skills/test_ovos.py, both failing before the fix
(['red', 'blue'] != ['rojo', 'azul']; voc_match on "quiero rojo" False) and
passing after. Unit suite: 701 passed, 1 pre-existing failure
(test_scheduler_delegation, also red on dev); end2end 13 passed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
The results of your automated verification are here! 📜I've aggregated the results of the automated checks for this PR below. 📡 Channel Compat — stableI've finished the digital walk-through of your PR. 🚶♂️ 🚧 Channel unresolvable with this checkout — the repo's dependency floors exceed what the channel pins (fleet finding; see the install log). Constraints: https://raw.githubusercontent.com/OpenVoiceOS/OpenVoiceOS/main/constraints-stable.txt 📡 Channel Compat — testingA detailed summary of the latest automation run. 📝 🚧 Channel unresolvable with this checkout — the repo's dependency floors exceed what the channel pins (fleet finding; see the install log). Constraints: https://raw.githubusercontent.com/OpenVoiceOS/OpenVoiceOS/main/constraints-testing.txt 📋 Repo HealthI've checked the repo's eyesight (aka observability). 👓 ✅ All required files present. Latest Version: ✅ ⚖️ License CheckChecking for any restrictive patent clauses. 📜 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintI've tidied up the results for you. 🧹 ❌ ruff: issues found — see job log 🔒 Security (pip-audit)Checking for any potential security breaches. 🔓 ✅ No known vulnerabilities found (72 packages scanned). 🔨 Build TestsChecking the alignment of our build components. 📏
❌ 3.10: Install OK, tests failed A robot's work is never done... but this PR check is! ⚙️ |
1 participant
Fixes #610.
voc_listused itslangargument only for the cache key and read the vocabulary fromself.resources, which is bound toself.lang. With the skill inen-USandcolour.vocshipped for bothen-US(red, blue) andes-ES(rojo, azul),voc_list("colour", "es-ES")returned['red', 'blue'], sovoc_match("quiero rojo", "colour", lang="es-ES")was False andvoc_match("I want red", "colour", lang="es-ES")was True.The change is one line: the vocabulary is loaded from
self.load_lang(lang=lang), the skill's existing per-languageSkillResourcesloader (self.resourcesis the same call withself.lang). The per-language cache key is unchanged, so repeated calls still hit the cache.test_voc_listandtest_voc_matchintest/unittests/skills/test_ovos.py(previously TODO placeholders) build a skill with both locale files and assert the es-ES list, the en-US list before and after an es-ES call, and the twovoc_matchoutcomes. With the source change reverted both tests fail (['red', 'blue'] != ['rojo', 'azul'];False is not true); with it applied both pass.Unit suite: 701 passed, 1 failed (
test_scheduler_delegation::test_a_delay_in_seconds_becomes_a_relative_timing,KeyError: 'owner'), and that one failure reproduces on a fresh clone of origin/dev at 09050fb in the same venv, so it is not from this change. End-to-end suite: 13 passed.