fix: handle absent number/date without False sentinel or crash - #85
fix: handle absent number/date without False sentinel or crash#85JarbasAl wants to merge 1 commit into
Conversation
handle_numbers guarded with `if number is not None`, which is truthy for extract_number's False sentinel, so an unparsed number leaked into number_trivia() and spoke a NUMBER_FACT_False dialog instead of falling back to random trivia. Guard on truthiness like the sibling handlers already do. handle_date unconditionally indexed extract_datetime()'s result, which raises TypeError when nothing is extractable (e.g. "tell me a date fact"), surfacing as skill.error instead of degrading to a random date fact. Fail-before: both new regression tests in test/unittests/test_random_fallback_handlers.py fail against the pre-fix code (NUMBER_FACT_False leaks through; handle_date raises TypeError: 'NoneType' object is not subscriptable) and pass after the fix. Full suite: 43 passed.
|
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 |
Tada! The results of the latest automation run are here. 🎉I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthI've checked the repo's hydration (aka documentation density). 💧 ✅ All required files present. Latest Version: ✅ 🌍 Locale BuildProcessing complete! Details follow. 📬 ✅ Locale properly configured (114 files, 10 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 228 locale files included in package 🔍 LintI've tidied up the results for you. 🧹 ❌ ruff: issues found — see job log 🎙️ SkillIs it a bird? Is it a plane? No, it's a skill check result! 🦸 ℹ️ Not an OVOS skill repo — check skipped. 🏷️ Release PreviewThe release train is fueling up! 🚂 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🧪 Spec LintChecking if there's anything else we need to do. 📋 12 error(s), 10 warning(s) 📊 CoverageExploring the coverage frontier of your PR. 🚀 ❌ 48.2% total coverage Per-file coverage (2 files)
Full report: download the 🔒 Security (pip-audit)Checking the vault for any security leaks. 🔓 ✅ No known vulnerabilities found (78 packages scanned). ⚖️ License CheckI've audited the 'About' files for accuracy. 📄 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔨 Build TestsThe build bots have finished their assembly. 🤖 ✅ All versions pass
🔌 Skill Tests (ovoscope)I've performed a deep-tissue massage on your skill's logic. 💆♂️ ❌ 0/29 passed, 29 errors ❌ **TestDateTrivia** — 0/1
❌ **TestMathTrivia** — 0/1
❌ **TestNumberTrivia** — 0/3
❌ **TestTriviaRoutingWithoutAdapt** — 0/4
❌ **TestYearTrivia** — 0/1
❌ **test_golden_utterance[fact about december 3]** — 0/1
❌ **test_golden_utterance[fact about the date 5]** — 0/1
❌ **test_golden_utterance[fact about the year 1992]** — 0/1
❌ **test_golden_utterance[fact about today]** — 0/1
❌ **test_golden_utterance[give me a fact about the number 42]** — 0/1
❌ **test_golden_utterance[give me a math fact]** — 0/1
❌ **test_golden_utterance[give me a mathematical trivia]** — 0/1
❌ **test_golden_utterance[give me a trivia about the year 1999]** — 0/1
❌ **test_golden_utterance[number fact 7]** — 0/1
❌ **test_golden_utterance[tell me a fact about mathematics]** — 0/1
❌ **test_golden_utterance[tell me a random number fact]** — 0/1
❌ **test_golden_utterance[year fact 2001]** — 0/1
❌ **test_negative_confusable_not_claimed[play a random song]** — 0/1
❌ **test_negative_confusable_not_claimed[play some music]** — 0/1
❌ **test_negative_confusable_not_claimed[set a timer for 5 minutes]** — 0/1
❌ **test_negative_confusable_not_claimed[tell me a random joke]** — 0/1
❌ **test_negative_confusable_not_claimed[what year is it]** — 0/1
❌ **test_negative_confusable_not_claimed[what's the weather]** — 0/1
❌ **test_negative_confusable_not_claimed[what's today's date]** — 0/1
🚌 Bus CoverageEnsuring the bus logic is battle-tested. ⚔️ Transmitted from the OVOS mothership. 🛸 |
1 participant
This fixes two handler bugs surfaced by skills-QA leads.
handle_numbersguarded onif number is not None, butextract_numberreturnsFalse(notNone) when nothing is found. SinceFalse is not Noneis true, an unparsed number was forwarded straight intonumber_trivia(False), producing a spoken sentinel like a "False" fact instead of falling back to random trivia. The fix switches the guard to plain truthiness, matching how the siblinghandle_mathandhandle_yearhandlers already treat this case.handle_dateunconditionally indexed the result ofextract_datetime(...), but that call returnsNonewhen no date can be extracted from the utterance. A bare request such as "tell me a date fact" would then raiseTypeError: 'NoneType' object is not subscriptableand surface as a skill error instead of degrading gracefully to a random date fact.Both bugs have regression tests in
test/unittests/test_random_fallback_handlers.py. I confirmed each test fails against the pre-fix code (aNUMBER_FACT_Falsesentinel leaks through in one case, aTypeErroris raised in the other) and passes after the fix, using a patch-revert-run-restore cycle rather than trusting the test in isolation. The full suite (43 tests, unit + end-to-end) passes after the fix.