fix: guard optional None blacklisted_skills/intents (SESSION-1 §3)#780
Conversation
blacklisted_skills/blacklisted_intents are OPTIONAL SESSION-1 fields and are legally absent (None) when a Session does not round-trip an empty list through SessionManager. The stop, fallback, converse and intent services tested membership/iteration directly (`x in sess.blacklisted_skills`), raising TypeError: 'NoneType' object is not iterable and aborting the stop ping/pong cascade, the fallback query cycle and the converse round-trip before any bus traffic. Guard each use with `(... or [])`. Surfaced by live integration testing in the OVOS spec-compliance harness (ovos-bus-client 2.x leaves these fields None); validated end-to-end there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughBlacklist membership checks in utterance matching, converse handling, fallback selection, and stop-skill collection now default missing blacklist lists to empty lists. ChangesIntent-service blacklist checks
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
I've completed my sweep! Here's the situation. 🧹I've aggregated the results of the automated checks for this PR below. 🌍 Locale BuildJust a quick heads-up on the latest check. 🛎️ ✅ Locale properly configured (64 files, 17 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 31 locale files included in package 📋 Repo HealthEnsuring the repo's immune system is strong (aka security checks). 🛡️ ✅ All required files present. Latest Version: ✅ 🔒 Security (pip-audit)I've audited the access control lists. 📋 ✅ No known vulnerabilities found (109 packages scanned). 🏷️ Release PreviewI've checked the release assets for completeness. 💾 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
📊 CoverageEnsuring every change is backed by a test. ✅ Files below 80% coverage (8 files)
Full report: download the 🔨 Build TestsChecking the blueprint against the actual construction. 📐 ✅ All versions pass
⚖️ License CheckThe license report is filed and ready for review. 📁 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔌 Skill Tests (ovoscope)Ensuring the skill's responses are natural and intuitive. 🗣️ ✅ 2/36 passed ❌ **TestAdaptIntent** — 0/4
❌ **TestCancelIntentMidSentence** — 0/1
❌ **TestConverse** — 0/1
❌ **TestCountSkills** — 0/4
❌ **TestDeactivate** — 2/3
❌ **TestFallback** — 0/1
❌ **TestGlobalStopVocWithActiveSkill** — 0/1
❌ **TestGlobalStopVocabulary** — 0/2
❌ **TestIntentPipelineRouting** — 0/4
❌ **TestLangDisambiguation** — 0/4
❌ **TestNoSkills** — 0/2
❌ **TestPadatiousIntent** — 0/4
❌ **TestStopNoSkills** — 0/3
❌ **TestStopServiceAsSkill** — 0/1
❌ **TestStopSkillCanHandleFalse** — 0/1
🚌 Bus CoverageChecking the insulation of our event handlers. 🏠 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
) blacklisted_skills/blacklisted_intents are OPTIONAL SESSION-1 fields and are legally absent (None) when a Session does not round-trip an empty list through SessionManager. The stop, fallback, converse and intent services tested membership/iteration directly (`x in sess.blacklisted_skills`), raising TypeError: 'NoneType' object is not iterable and aborting the stop ping/pong cascade, the fallback query cycle and the converse round-trip before any bus traffic. Guard each use with `(... or [])`. Surfaced by live integration testing in the OVOS spec-compliance harness (ovos-bus-client 2.x leaves these fields None); validated end-to-end there. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
blacklisted_skills/blacklisted_intentsare optional SESSION-1 §3 fields and are legallyNonewhen a Session doesn't carry an empty list. The stop / fallback / converse / intent services tested membership directly (x in sess.blacklisted_skills) →TypeError: 'NoneType' object is not iterable, aborting the stop ping/pong cascade, the fallback query cycle, and the converse round-trip before any bus traffic.Guards every use with
(... or [])(7 call sites across stop_service / fallback_service / converse_service / service.py).How it was found: live integration testing in the OVOS spec-compliance harness (ovos-bus-client 2.x leaves these fields
None); the harness conformance suite went green once both this and the padacioso lru_cache fix (OpenVoiceOS/padacioso#60) landed.🤖 Generated with Claude Code
Summary by CodeRabbit