fix: move Wolfram intent suppression to native <intent>.blacklist locale files - #133
fix: move Wolfram intent suppression to native <intent>.blacklist locale files#133JarbasAl wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Analyzing your contribution... results ready! 🧪I've aggregated the results of the automated checks for this PR below. 🔍 LintEnsuring the codebase remains stable and healthy. 🛡️ ❌ ruff: issues found — see job log 🔒 Security (pip-audit)Evaluating the security posture of your changes. 🛡️ ✅ No known vulnerabilities found (76 packages scanned). 🏷️ Release PreviewEvaluating the excitement level for the next release. 🤩 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🔌 Skill Tests (ovoscope)Checking for any potential conversational dead ends. 🚧 ✅ 19/19 passed ✅ TestWolfieIntents — 1/1 🚌 Bus CoverageHow deep does the message bus rabbit hole go? 🐇 ⚖️ License CheckScanning for any potential trademark infringements. ™️ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 📋 Repo HealthScanning for any signs of 'orphaned' code limbs. 🦾 ✅ All required files present. Latest Version: ✅ 🎙️ SkillEvaluating the skill's potential for impact. 🌟 🎙️ ovos-skill-wolfie.openvoiceos — 10 languages en-US: 1 intents · 2 vocab · 1 dialogs · skill.json ✅ Translation coverage — 9 languages (1 complete, 8 partial)
🔨 Build TestsChecking if the gears are still turning smoothly... ⚙️ ✅ All versions pass
🧪 Spec LintThe automated sentinel is back with news. 💂♂️ 13 error(s), 12 warning(s) 📊 CoverageA deep dive into the sea of test results. 🌊 ✅ 94.3% total coverage Per-file coverage (2 files)
Full report: download the May the tests be ever in your favor! 🏹 |
…ale files ovos-workshop's register_intent_file loads a sibling "<intent-base>.blacklist" locale file into blacklisted_words natively (landed pre-9.1.0a1, confirmed against origin/dev source). This replaces the skill's voc_blacklist=["MiscBlacklist"] wiring with that mechanism: MiscBlacklist.voc is renamed to search_wolfie.blacklist (matching the intent file's base name) in every locale that had it (ca-ES, en-US, es-ES, fr-FR, gl-ES), in both the top-level locale/ mirror and ovos_skill_wolfie/locale/, and the voc_blacklist argument is dropped from the @intent_handler decorator. The floor pin on ovos-workshop is bumped to >=9.1.0a1 to require the feature. Padacioso's exclude_keywords matches single-word blacklist entries on whole-word boundaries, so "installment" no longer collides with the blacklisted word "install" - the over-suppression bug in the old substring-based voc_match check. Added regression coverage: an intent-level calc_intent assertion and a bus-round-trip golden-utterance test confirming the blacklisted phrase reaches no wolfie handler while "installment loan" still matches. Fail-before: with the new search_wolfie.blacklist files removed, test_en_us_wolfram_intents and test_blacklisted_phrase_reaches_no_wolfie_handler both fail (AssertionError: 'ask wolfram can you install skills' matches search_wolfie.blacklist but was claimed by ovos-skill-wolfie.openvoiceos); restoring the files makes both pass again. Full suite: 54 passed (dev baseline 52, no regressions). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
34c8df3 to
0aee9b0
Compare
1 participant
This replaces #123's hand-rolled Python suppression logic with the native mechanism ovos-workshop already provides:
register_intent_fileloads a sibling<intent-base>.blacklistlocale file directly intoblacklisted_words, novoc_blacklist=[...]decorator argument required.MiscBlacklist.vocis renamed tosearch_wolfie.blacklistin every locale that had it (ca-ES, en-US, es-ES, fr-FR, gl-ES), in both the top-levellocale/mirror andovos_skill_wolfie/locale/, with the phrases unchanged. Thevoc_blacklist=["MiscBlacklist"]argument is dropped from the@intent_handlerdecorator onhandle_search, and the ovos-workshop floor pin moves to>=9.3.0a1,<10.0.0. Version 9.1.0a1 first loads the .blacklist file but only the legacy padatious path receives it; 9.3.0a1 is the first version whose spec-message emission wires suppression into both padatious and padacioso, hence the floor.Because the suppression now happens inside the padatious/padacioso container registration itself rather than as a separate
voc_matchgate the skill runs before speaking, an utterance that fails the blacklist check never gets routed to this skill's handler in the first place, which also means the utterance stays available for other skills' pipelines to claim instead of dead-ending here.Both padatious and padacioso enforce word-boundary suppression on single-word blacklist entries via the spec registration message, so
installmentno longer collides with the blacklisted wordinstall— that was the over-suppression bug in #123's version. Test coverage was extended accordingly:test_intents_en_us.pynow checkscalc_intentreturns no match for a blacklisted phrase and still matches"what is an installment loan", and a new bus-round-trip test intest_golden_utterances.pyconfirms the blacklisted utterance reaches no wolfie handler end to end while the boundary case still routes correctly.Open PR #126 adds a
da-DKMiscBlacklist.vocfor the old mechanism; it will need ada-DK/search_wolfie.blacklistinstead once this lands, but that repo/file is left untouched here.