feat: manifest side of ovos.skills.list (OVOS-INTENT-4 §8.6/§10.3) - #970
Draft
JarbasAl wants to merge 1 commit into
Draft
feat: manifest side of ovos.skills.list (OVOS-INTENT-4 §8.6/§10.3)#970JarbasAl wants to merge 1 commit into
JarbasAl wants to merge 1 commit into
Conversation
IntentManifest subscribes to ovos.skill.loaded (OVOS-INTENT-4 §8.6)
and indexes (session_id, skill_id) -> capabilities, reading the
session from context.session.session_id exactly as _session_id_of
does for intent registrations. ovos.skill.deregister drops the
announcement alongside the registrations it already removes.
A new ovos.skills.list handler answers over message.response (so the
topic derives as ovos.skills.list.response per OVOS-MSG-1 §5.3) with
{"ok": true, "skills": [...]}, each entry carrying skill_id,
session_id, capabilities and intents (the manifest row count for that
scope). The session_id filter follows §10.3/§11.2: "default" plus the
named session, or every session when omitted. Unknown capability
names are dropped at announcement time per the §8.6 vocabulary. An
empty index answers an empty list, never silence. Entries sort with
"default" first, then skill_id.
ovos.skills.fallback.list and its FallbackService registry were
already absent from dev, so no removal was needed there.
Fail-before: reverting only the manifest.py change and running the
new TestSkillsListManifest class fails all 7 tests with
AttributeError ('IntentManifest' object has no attribute
'_on_skill_loaded'/'_on_skills_list'); restoring the change passes
all 7, and the full unittest suite (623 tests) and the
test_intent_pipeline.py end2end suite (4 tests) remain green.
Co-Authored-By: Claude Sonnet 5 <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 |
Just the facts, ma'am. Here's your report. 👮♂️I've aggregated the results of the automated checks for this PR below. 📚 DocsChecking the status... yep, it's done! ✅ ✅ All required documentation files present. ✅ 🔒 Security (pip-audit)Ensuring our code is secure by design. 📐 ✅ No known vulnerabilities found (113 packages scanned). 🔎 Type CheckChecking the alignment of your contribution. 📏 ❌ mypy: 367 error(s) found
Errors (showing first 10/367)📡 Channel Compat — stableChecking if everything is still on track. 🛤️ 🚧 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 🌍 Locale BuildThe latest check report is officially filed. 📁 ✅ Locale properly configured (65 files, 18 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 32 locale files included in package 🔌 Plugin DetectionEnsuring the plugin remains a valuable addition to OVOS. 💎 ❌ Plugin Status: ERRORS (1) Plugin Info:
OPM Detection:
Entry Point Validation:
⊘ No Issues:
📊 CoverageEvaluating the thoroughness of our test suite. 🔎 Files below 80% coverage (10 files)
Full report: download the 🔨 Build TestsChecking if the code is ready for prime time. 📺 ✅ All versions pass
🔌 Skill Tests (ovoscope)Ensuring the skill provides a consistent and delightful experience. 💖 ✅ 18/42 passed ✅ TestActiveHandlersOnlyIsNotPolled — 1/1 ❌ **TestAdaptIntent** — 0/4
❌ **TestCancelIntentMidSentence** — 0/1
❌ **TestConverse** — 0/1
❌ **TestDeactivate** — 2/3
❌ **TestFallback** — 0/1
❌ **TestIntentPipelineRouting** — 0/4
❌ **TestLangDisambiguation** — 0/4
❌ **TestLegacyIntentIdBackCompat** — 0/2
❌ **TestNoSkills** — 0/2
❌ **TestPadatiousIntent** — 0/4
🚌 Bus CoverageA deep dive into the sea of message bus results. 🌊 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
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.
IntentManifestnow indexesovos.skill.loadedannouncements and answersovos.skills.list, implementing the manifest side of OVOS-INTENT-4 §8.6/§10.3 (architecture#251, 887f248, approved). The reader needs to decide whether theovos.skill.loadedproducer side (ovos-workshop#623, already open) should merge first — this PR's consumer is inert without it, since no producer ondevcurrently emits that event.Governing clause, §10.3: "
session_idis an optional filter. When present the response carries the skills loaded in the effective scope of that session: those announced underdefaultplus those announced under thatsession_id... When absent the response carries every announced skill under every session." Each entry carriesskill_id,session_id,capabilities, andintents(the manifest row count for that scope); an empty index answers{"ok": true, "skills": []}.The response is built with
message.response(...)so the topic derives asovos.skills.list.responseper OVOS-MSG-1 §5.3.ovos.skill.deregisterdrops the announcement alongside the intent registrations it already removes. Unknown capability names are dropped at announcement time rather than surfaced or rejected, per the §8.6 vocabulary (fallback,common_query,converse). Entries sort withdefaultfirst, thenskill_id, matching the existingovos.intent.describeordering convention.ovos.skills.fallback.listand itsFallbackServiceregistry were checked against currentdevand are already absent — there was nothing to remove there.skillmanager.list/mycroft.skills.listare untouched.Verified against source: read the full
ovos_core/intent_services/manifest.pyon this branch'sdevbase before editing, confirmed_session_id_of/raw_session_idbehavior by inspection, and confirmedMessage.responsederives<topic>.responseby readingovos_bus_client.message.Message.response's source directly in the installed environment. Test-extra install (uv pip install -e ".[test]") confirmed by checking matcher plugins load and end2end fixtures collect.Seven new tests in
test/unittests/test_intent_manifest.py::TestSkillsListManifestassert values (capability lists, intent counts, exact session-id sets, ordering) rather than structure. Reverting only themanifest.pyhunk and running that class fails all 7 withAttributeError: 'IntentManifest' object has no attribute '_on_skill_loaded'; restoring it passes all 7. Fulltest/unittestssuite: 623 passed.test/end2end/test_intent_pipeline.py: 4 passed, 8 subtests passed (single run, no flake observed).