fix: conform transformer chains to OVOS-TRANSFORM-1#785
Draft
JarbasAl wants to merge 2 commits into
Draft
Conversation
📝 WalkthroughWalkthrough
ChangesCancellation metadata
Transformer dispatch identity
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 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 |
Beep boop! Data processing finished. ⚙️I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthA quick checkup for the repository! 🩺 ✅ All required files present. Latest Version: ✅ 🏷️ Release PreviewThe release tag is ready to be minted. 🏷️ Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🌍 Locale BuildEverything looks good so far! ✅ ✅ Locale properly configured (64 files, 17 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 31 locale files included in package 🔒 Security (pip-audit)Ensuring our security logs are being captured. 🪵 ✅ No known vulnerabilities found (110 packages scanned). 📊 CoverageThe coverage audit is ready for your inspection. 📋 Files below 80% coverage (9 files)
Full report: download the 🔨 Build TestsEnsuring the foundation is solid for these changes. 🏛️
❌ 3.10: Build OK, install failed ⚖️ License CheckEnsuring our project remains legally compliant. ✅ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔌 Skill Tests (ovoscope)Testing the skill's 'long-term memory' (aka settings/state). 🧠 ✅ 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
❌ **TestStopServiceNotASkill** — 0/1
❌ **TestStopSkillCanHandleFalse** — 0/1
🚌 Bus CoverageEnsuring every intent and event is reached by the test suite. 🎯 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
JarbasAl
force-pushed
the
fix/transform-1-conformance
branch
from
July 3, 2026 23:28
31dd04e to
ed394c1
Compare
JarbasAl
marked this pull request as ready for review
July 16, 2026 07:21
Bring the utterance / metadata / intent transformer chains into conformance with the OVOS-TRANSFORM-1 specification: - §4 ordering: the chain MUST run in ascending priority order (lower number = earlier). All three services sorted with reverse=True (descending); drop reverse so priority 1 runs before 50 runs before 100, matching the spec and the fallback-skill convention. - §1.3 provenance: stamp each touching transformer's id as the last element of the corresponding <type>_transformer_ids context list. - §3.4 / §9 intent identity: reject an intent transformer return whose dispatch identity (match_type / skill_id) differs from its input, treating it as a §7 shape violation and keeping the prior Match. - §8.1 cancel_by: when a transformer signals cancellation, the orchestrator stamps cancel_by from the emitting transformer's id (not from any payload value), stops the chain, and surfaces cancel_reason / cancel_by on ovos.utterance.cancelled (§8.2). - §7 wrong-shape returns: reject wrong-shape transformer output (wrong type / arity, non-dict context, non-Match intent) instead of silently unpacking it; keep the prior output unchanged. Extends test/unittests/test_transformers.py with conformance tests for each MUST and flips the existing priority-ordering assertions from descending to ascending. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…VOS-TRANSFORM-1 conformance dev's transformer chains already subclass ovos-plugin-manager's shared TransformersService bases. This PR's OVOS-TRANSFORM-1 conformance work moved into ovos-plugin-manager#417 (§1.3 provenance, §7 wrong-shape rejection, §3.4 intent identity invariant); ovos_core/transformers.py stays as dev's thin subclasses. - Bump the ovos-plugin-manager floor to >=2.10.3a1 (next prerelease containing #417's conformance work). - Adapt test_transformers.py's TestIntentTransformersServiceTransform to the now-enforced §3.4 identity invariant: a Match that changes match_type is correctly rejected rather than passed through; add a dedicated rejection test and an identity-preserving enrichment test. - Add §8.2 coverage in test_intent_service_extended.py for ovos.utterance.cancelled surfacing cancel_reason/cancel_by from the §8.1 signal (core-only wiring, not duplicated by the shared bases).
JarbasAl
force-pushed
the
fix/transform-1-conformance
branch
from
July 23, 2026 21:59
ed394c1 to
7055e06
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/unittests/test_transformers.py`:
- Around line 367-390: The dispatch-identity tests currently only exercise
match_type because both fixtures use skill_id=None. Update
test_transform_accepts_enriched_match and
test_transform_rejects_dispatch_identity_change to use a non-null original
skill_id, and add/assert a transformed match with a different skill_id is
rejected while the original match is retained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fa097521-65b8-408d-b036-64880c84f4ac
📒 Files selected for processing (4)
ovos_core/intent_services/service.pypyproject.tomltest/unittests/test_intent_service_extended.pytest/unittests/test_transformers.py
Comment on lines
+367
to
+390
| original = _make_intent_match("original:intent") | ||
| modified = IntentHandlerMatch( | ||
| match_type="original:intent", | ||
| match_data={"enriched": True}, | ||
| skill_id=original.skill_id, | ||
| utterance="hello", | ||
| ) | ||
| p = _make_mock_plugin("p", priority=50) | ||
| p.transform.return_value = modified | ||
| svc = _make_intent_service(plugins=[p]) | ||
| result = svc.transform(original) | ||
| self.assertEqual(result.match_data, {"enriched": True}) | ||
|
|
||
| def test_transform_rejects_dispatch_identity_change(self): | ||
| """OVOS-TRANSFORM-1 §3.4: a returned Match whose match_type/skill_id | ||
| differs from its input is a §7 shape violation -- discarded, prior | ||
| Match kept.""" | ||
| original = _make_intent_match("original:intent") | ||
| modified = _make_intent_match("modified:intent") | ||
| p = _make_mock_plugin("p", priority=50) | ||
| p.transform.return_value = modified | ||
| svc = _make_intent_service(plugins=[p]) | ||
| result = svc.transform(original) | ||
| self.assertEqual(result.match_type, "modified:intent") | ||
| self.assertEqual(result.match_type, "original:intent") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover skill_id in the dispatch-identity tests.
Both fixtures use skill_id=None, so these tests only validate match_type identity. Add a case with a non-null original skill and a transformed match whose skill_id changes, asserting that the original match is retained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/unittests/test_transformers.py` around lines 367 - 390, The
dispatch-identity tests currently only exercise match_type because both fixtures
use skill_id=None. Update test_transform_accepts_enriched_match and
test_transform_rejects_dispatch_identity_change to use a non-null original
skill_id, and add/assert a transformed match with a different skill_id is
rejected while the original match is retained.
JarbasAl
marked this pull request as draft
July 23, 2026 22:22
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
ovos-coreinto conformance with OVOS-TRANSFORM-1. Originallyreimplemented the utterance / metadata / intent transformer chains locally in
ovos_core/transformers.py;devhas since refactored those services tosubclass the shared
TransformersServicebases inovos-plugin-manager, sothis PR was reworked to move the conformance logic there instead
(
ovos-plugin-manager#417, see "Depends on" below) and keep only thecore-only pieces here:
What's in this PR
ovos.utterance.cancelledpayload.IntentService.send_cancel_eventnow surfaces
cancel_reason/cancel_by(stamped by the §8.1 cancellationsignal) on the emitted
ovos.utterance.cancelledmessage, when present.ovos-plugin-managerfloor bump to>=2.10.3a1— the next prereleasethat will contain the shared-base conformance work.
test_intent_service_extended.pygains §8.2 coverage forcancel_reason/cancel_bypropagation (and their absence).test_transformers.py's intent-transform test was adapted to the nowshared-base-enforced §3.4 identity invariant — a
Matchthat changesmatch_typeis correctly rejected rather than silently passed through; adedicated rejection test and an identity-preserving enrichment test were
added.
What moved to ovos-plugin-manager#417
ovos_core/transformers.pystays asdev's thin subclasses of the sharedbases; §1.3 provenance stamping, §7 wrong-shape rejection, and the §3.4
intent dispatch-identity invariant now live in
ovos_plugin_manager/transformer_services.pyso every consumer of theshared bases gets them, not just
ovos-core. Priority-ascending ordering,the
orderconfig override, and §8.1 cancellation stamping were alreadyconformant in the shared bases.
Depends on ovos-plugin-manager#417 — must merge first (and this PR's
floor-pin bump land in a release) before this PR is testable against a
released
ovos-plugin-manager.Testing
test/unittests/suite run locally withovos-plugin-manager#417'sbranch installed editable: 304 passed, no regressions vs. clean
dev.ovos-plugin-manager#417(
test_transform_returns_modified_intentasserting a rawmatch_typeswap passes through) was a stale test exercising behavior the spec
disallows; fixed as described above rather than worked around.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes