feat: OVOS-PIPELINE-1 conformance (matched/unmatched/handled, updated_session, active_handlers)#778
Draft
JarbasAl wants to merge 8 commits into
Draft
feat: OVOS-PIPELINE-1 conformance (matched/unmatched/handled, updated_session, active_handlers)#778JarbasAl wants to merge 8 commits into
JarbasAl wants to merge 8 commits into
Conversation
…_session, pipeline_id)
Bring the intent-service orchestrator into line with OVOS-PIPELINE-1:
- §4.2 updated_session: _emit_match_message now RETURNS the working session
(after applying Match.updated_session) and handle_utterance adopts it back
into message.context["session"], so a plugin's match-phase session mutation
reaches the default-session sync and every downstream stage (previously the
adopted snapshot only reached the dispatch reply and was discarded from
SessionManager).
- §9.2 ovos.intent.matched: broadcast (via SpecMessage.INTENT_MATCHED) before
every dispatch, carrying {skill_id, intent_name, lang, utterance, slots,
pipeline_id}.
- §9.3 ovos.intent.unmatched: send_complete_intent_failure now emits the spec
topic SpecMessage.INTENT_UNMATCHED. This topic is in ovos-spec-tools
MIGRATION_MAP (complete_intent_failure -> ovos.intent.unmatched), so the bus
bridge re-delivers legacy complete_intent_failure transparently; no hand-rolled
dual-emit.
- §3.1/§7.1 pipeline_id stamping: the matching stage id is stamped on the
dispatch context so downstream Messages are attributable to the plugin, and
surfaces in the matched payload.
- Cancellation terminal path now uses SpecMessage.UTTERANCE_CANCELLED /
UTTERANCE_HANDLED constants.
- Pin ovos-spec-tools>=0.11.0a1 (floor where INTENT_UNMATCHED is in MIGRATION_MAP).
Tests updated to assert the spec topics (the bus bridge keeps legacy working):
no-match sequences assert ovos.intent.unmatched; matched-path sequences assert
the new ovos.intent.matched (or filter it via ignore_messages where the test
asserts routing, not the notification).
Deferred (out of scope here): session.active_handlers (§7.1) — the bus-client
Session model stores active_skills as [skill_id, ts] pairs, not the spec's
{skill_id, activated_at, ...} objects; that field shape is an ovos-bus-client
change. The §8 handler-lifecycle trio + the matched-path ovos.utterance.handled
are still emitted by ovos-workshop's skill base, not the orchestrator; relocating
them to the orchestrator is a separate cross-repo refactor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 returned from the depths of the test suite with news. 🤿I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthScanning for any signs of 'merge conflict' stress. 😫 ✅ All required files present. Latest Version: ✅ 🔒 Security (pip-audit)Scanning for any potential man-in-the-middle risks. 👨💻 ✅ No known vulnerabilities found (109 packages scanned). 🏷️ Release PreviewI've checked the 'Known Issues' list for honesty. 😇 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🌍 Locale BuildThe automated report has been generated. 🖨️ ✅ Locale properly configured (64 files, 17 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 31 locale files included in package 📊 CoverageMeasuring the safety net for your changes. 🥅 Files below 80% coverage (8 files)
Full report: download the 🔨 Build TestsThe build report has been filed and is ready. 📁 ✅ All versions pass
⚖️ License CheckVerifying the source of all binary files. 💾 ✅ 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 remains engaging and helpful. 😊 ❌ 2/36 passed, 10 failed ❌ **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 CoverageMeasuring the reach of our bus handlers. 📏 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
…ession
On dispatch, push the spec {skill_id, activated_at} record onto the working
session via Session.add_active_handler (ovos-bus-client #234), replacing the
legacy activate_skill (now a back-compat shim over the same helper, with
active_skills a projection of active_handlers). The push is applied after
Match.updated_session is committed so the dispatched skill_id lands head-first,
and is SUPPRESSED for reserved intent_name dispatches per §7.1/§7.3
(converse/response/stop/fallback/common_query), keyed off the reserving
pipeline-plugin role.
Adds tests asserting the push on a normal dispatch, prior-same-skill eviction,
deactivation skip, and suppression for each reserved-name pipeline.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After a plugin returns a Match, the orchestrator now verifies the match's slot map contains every slot in the intent's required_slots (INTENT-3 §5.3). A required slot that is absent makes the orchestrator treat the match as if the plugin had declined and continue iteration to the next pipeline stage — no bus event is emitted (§6.2 observable semantics, same as the §5.3/§5.4 denylist backstop it runs after). required_slots is sourced from the Match itself (match_data['__required_slots__']) — the only conformant in-process channel a plugin has today, since ovos-core does not yet maintain an INTENT-4 §10 registration manifest in the orchestrator. When a plugin does not surface required_slots the backstop is a no-op and engine-side enforcement stays authoritative. Full manifest plumbing is a follow-up (TODO in _missing_required_slots). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`Session.blacklisted_skills`/`blacklisted_intents` can be None (ovos-bus-client #234 Session no longer coerces them to []), causing `TypeError: argument of type 'NoneType' is not iterable` in the match loop's denylist membership checks — which crashed before the OVOS-PIPELINE-1 §6.2 required_slots backstop could run. Treat a None blacklist as empty. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nstall_pip The reusable ovoscope workflow's CI-fallback path installs the stale PyPI ovos-adapt-parser / ovos-padatious, whose pipeline plugins pin ovos-workshop<8.0.0 and conflict with the installed workshop 8.3.0a1, leaving the real opm.pipeline entry points absent and failing the required-pipeline verification. Force-reinstall the workshop-decoupled adapt/padatious pipeline plugins from their fix branches via pre_install_pip (runs after plugin install, before verification). Temporary until adapt#45 + padatious#73 merge to dev. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ovoscope suite is a full-stack integration test (it asserts the complete emitted message sequence, which spans the workshop INTENT-4 producer, bus-client session behavior and the pipeline engines). It greens on dev once the spec stack lands in merge order, not on a standalone conformance PR, so the temporary plugin git-deps and reusable-workflow pin are removed to keep this PR clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Brings the intent-service orchestrator (
ovos_core/intent_services/service.py) into line with OVOS-PIPELINE-1.What's done
Match.updated_sessionreaches downstream state. Previously the snapshot was adopted only into the dispatch reply; for the default session it was discarded fromSessionManager(the end-of-handle_utterancesync reserialized the pre-match session)._emit_match_messagenow returns the working session andhandle_utterancewrites it back intomessage.context["session"], so a plugin's match-phase mutation reaches the default-session sync and every downstream stage.ovos.intent.matched. Broadcast (SpecMessage.INTENT_MATCHED) before every dispatch, carrying{skill_id, intent_name, lang, utterance, slots, pipeline_id}. It's a notification, not a dispatch.ovos.intent.unmatched.send_complete_intent_failureemits the spec topicSpecMessage.INTENT_UNMATCHED. This topic is a 1:1 rename in ovos-spec-toolsMIGRATION_MAP(complete_intent_failure → ovos.intent.unmatched), so the bus bridge re-delivers legacycomplete_intent_failuretransparently — no hand-rolled dual-emit.pipeline_idstamping. The matching stage's id is stamped on the dispatchcontext["pipeline_id"]so every downstream Message is attributable to the plugin, and it surfaces in the matched payload.SpecMessage.UTTERANCE_CANCELLED/UTTERANCE_HANDLEDconstants.ovos-spec-tools>=0.11.0a1(the floor whereINTENT_UNMATCHEDis inMIGRATION_MAP).Tests updated to assert the spec topics (the bus bridge keeps legacy working): no-match sequences assert
ovos.intent.unmatched; matched-path sequences assert the newovos.intent.matched(or filter it viaignore_messageswhere the test asserts routing, not the notification).What's deferred (out of scope here)
session.active_handlerspush. The bus-clientSessionmodel storesactive_skillsas[skill_id, timestamp]pairs, not the spec'sactive_handlersarray of{skill_id, activated_at, ...}objects (OVOS-SESSION-1 §3 field table). Adding theactive_handlersfield + its{skill_id, activated_at}push (and the reserved-name suppression of §7.3) is an ovos-bus-client session-model change and belongs in a bus-client PR; this PR does not force a risky model change. The existingactivate_skill(...)recency tracking is preserved.ovos.utterance.handled. These are still emitted by ovos-workshop's skill base (_on_event_start/_on_event_end), not the orchestrator. The end-marker invariant (exactly oneovos.utterance.handledper utterance) holds today across cancel / no-match / matched paths. Relocating the trio + success-path end-marker into the orchestrator (which dispatches asynchronously over the bus and has no synchronous handler-return point) is a separate cross-repo refactor.Tests
python -m pytest test/unittests/→ 244 passed (incl. the updatedsend_complete_intent_failureassertion).LD_LIBRARY_PATHpointing at a local libfann build (the padatious plugin needslibdoublefann.so.2). Updated no-match / matched sequences pass (test_no_skills,test_lang_detect,test_activate, and the blacklist/no-match subtests oftest_adapt/test_padatious/test_converse). The success-match scenariostest_adapt_match,test_padatious_match,test_parrot_modefail identically on cleandev— a pre-existing skill-matching/fixture issue in the local MiniCroft, not introduced here. The fulltest/end2endrun does not finish within the local time budget (persona/converseget_responsetimeouts) on eitherdevor this branch; noFappears before the cap.🤖 Generated with Claude Code