test: intent-topic back-compat guards — in-process, plus a mixed-version two-venv matrix - #836
Open
JarbasAl wants to merge 6 commits into
Open
test: intent-topic back-compat guards — in-process, plus a mixed-version two-venv matrix#836JarbasAl wants to merge 6 commits into
JarbasAl wants to merge 6 commits into
Conversation
Relocated from ovoscope#127 at the maintainer's request: ovoscope is the harness library, not the stack, and a stack contract must be pinned where the stack lives. Pins the send-side half of the intent-topic migration that test_intent_alias_backcompat.py does not cover: when emit_legacy compat is enabled, a canonical intent dispatch must re-emit the .intent-suffixed twin exactly once, with identical data/context, to a bus-only legacy listener that never registered through this core (paired positive/negative control); with compat disabled, only the canonical dispatch is observed. The re-emit and FakeBus wiring do not exist yet, so those assertions are marked xfail(strict=True) pending the compat train: ovos-spec-tools#88 (IntentAliasRegistry / legacy_reemit_targets, vendored unreleased), bus-client#271, and ovos-utils#411. strict=True turns the day the train lands into a loud XPASS failure, the signal to drop the marker and promote the guard permanently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
The results of your automated verification are here! 📜I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthScanning for any signs of code rot or decay. 🍄 ✅ All required files present. Latest Version: ✅ 📚 DocsI've checked the pulse of your pull request. 💓 ✅ All required documentation files present. ✅ 🔎 Type CheckChecking the status... yep, it's done! ✅ ❌ mypy: 292 error(s) found
Errors (showing first 10/292)🏷️ Release PreviewI've checked the 'Documentation Updates' link. 📖 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🌍 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 📊 CoverageI've been crunching the numbers! Here's how the test coverage changed. 📈 Files below 80% coverage (9 files)
Full report: download the 🔒 Security (pip-audit)I've checked the vulnerability database for hits. 🎯 ✅ No known vulnerabilities found (113 packages scanned). ⚖️ License CheckI've verified the license compliance for your changes. ✅ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔨 Build TestsThe build is complete. No hard hats required. 👷♂️ ✅ All versions pass
🔌 Plugin DetectionThe OPM check is now finished. 🏁 ❌ Plugin Status: ERRORS (1) Plugin Info:
OPM Detection:
Entry Point Validation:
⊘ No Issues:
🔌 Skill Tests (ovoscope)A detailed report on the skill's intent-matching accuracy. 📝 ✅ 6/44 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
❌ **TestIntentLegacyReemitGuard** — 2/4
❌ **TestIntentPipelineRouting** — 0/4
❌ **TestLangDisambiguation** — 0/4
❌ **TestLegacyIntentIdBackCompat** — 0/2
❌ **TestNoSkills** — 0/2
❌ **TestPadatiousIntent** — 0/4
❌ **TestStopNoSkills** — 0/3
❌ **TestStopServiceNotASkill** — 0/1
❌ **TestStopSkillCanHandleFalse** — 0/1
🚌 Bus CoverageMeasuring the reach of our test cases across the message bus. 📏 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
…bus) The in-process guards in this repo share one interpreter, so they can only observe one set of package versions. The breakage they pin is cross-version by definition: a skill container frozen months ago talking to a stack that moved on. Seeing that needs two package sets alive at once, which means two venvs and a real websocket between them. Adds `test/backcompat/`: a real `ovos-messagebus` on a private port, a standalone skill launched from a second venv, and one parameterized script that takes the two venvs as inputs. `.github/workflows/backcompat_matrix.yml` runs the four skill/core combinations as four matrix entries. | combo | skill binds | core dispatches | handler runs | |--------------------|---------------|-----------------|--------------| | old skill/old core | suffixed only | suffixed | yes | | old skill/new core | suffixed only | canonical | **no** | | new skill/old core | both | suffixed | yes | | new skill/new core | both | canonical | yes (once) | Only `old skill/new core` is broken; it is xfail(strict=True) pending ovos-bus-client#271. The other three are passing controls that prove the harness can see a handler fire at all. Pins, chosen by behaviour and asserted at runtime so a release cannot silently turn a red cell green: * `ovos-workshop==9.3.1a2` — newest release binding the suffixed topic ONLY (9.3.2a1 added the canonical binding in #497 and hides the breakage); * `ovos-padatious==2.0.0a1` — newest release BEFORE registration-time canonicalization (`_dealias_intent_name`, 2.0.1a1). ovos-core forwards `match.match_type` verbatim, so the pipeline plugin decides the spelling. `ovos-bus-client` is deliberately unpinned on the skill side: a workshop pin is a floor, not a ceiling, so a rebuilt container resolves a current client. The suite asserts this, because #271's repair is receive-side and can only run in the process holding the stale binding. Keeps the existing in-process guards untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JarbasAl
changed the title
The alias registry was removed (spec-tools#92); the bridge is two stateless rules now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… cells Adds four channel-pinned cells alongside the existing boundary-pin cells: stable-skill/dev-core, dev-skill/stable-core, testing-skill/dev-core, and dev-skill/testing-core. Each is built from the OVOS distro's own constraints-stable.txt / constraints-testing.txt, fetched fresh at build time rather than vendored, so the gate tracks the fleet: a distro pin bump past a behavior boundary flips the affected cell red at that moment. As pinned today both channels float below the 9.3.2a1/2.0.1a1 boundaries, so the two skill-side channel cells hit the same known gap as old-skill/new-core and share its xfail(strict=True); the other two are passing controls. Also fixes a real setuptools<81 pkg_resources gap the channel venvs surfaced, and exempts channel combos from the "frozen container resolves a modern bus-client" assertion, since a distro constraints file caps bus-client too — a genuine fleet-inventory finding, not a broken assumption. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keeps the empirical pin-resolution finding, but describes the current design: emitter wire twin (reaches frozen images) + local inbound canonicalization (serves rebuilt containers), each cell noting which rule it exercises. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JarbasAl
marked this pull request as ready for review
August 1, 2026 18:03
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.
Summary
Two layers of guard for the intent-topic migration, in one branch:
test/end2end/test_intent_legacy_reemit_guard.py, relocated from ovoscope#127 per maintainer decision (ovoscope is the harness library, not the stack, so a stack contract does not belong pinned there);test/backcompat/, a four-cell matrix that runs two venvs against one realovos-messagebus.The second layer is new in this update and is the one that reproduces the actual field scenario.
Layer 1 — in-process guard (unchanged)
Paired positive/negative guard for the send-side half of the migration; the registration half is already covered by
test/end2end/test_intent_alias_backcompat.py:emit_legacyenabled, a canonical intent dispatch must re-emit the.intent-suffixed twin exactly once, with identical data/context, to a bus-only legacy listener that never registered through this core;FakeBusmust expose the same re-emit hook so this can run without a live MQ.None of that wiring exists yet, so those assertions are
xfail(strict=True)pending the compat train.Layer 2 — mixed-version back-compat matrix
Why the in-process guards are not enough
Every guard above shares one interpreter, so it can only ever observe one set of package versions. The breakage is definitionally cross-version: an outdated standalone skill process — frozen
ovos-workshop, as in a container built months ago — connects to a realovos-messagebusand the current stack dispatches at it. Two package sets have to be alive at once, which one process cannot do.Hence two venvs and a real websocket on localhost, no docker. A docker flavour can follow later as a manual job; venv isolation is the same experiment with a far cheaper CI story.
The matrix
Four combinations, one CI matrix entry each, all sharing one parameterized script that takes the two venv paths as inputs:
xfail(strict=True)Only one cell is broken. The other three are passing controls: they prove the harness can see a handler fire at all, so the red cell is a real finding and not a broken fixture.
new skill / new coredoubles as a double-fire guard — a skill bound to both spellings must not answer twice once the mirror is live.What actually moved
OVOS-MSG-1 §2.1.1 builds the dispatch topic as
<skill_id>:<intent_name>. Old workshop built it from the padatious resource filename, leaking.intentonto the wire. Then two independent changes landed:ovos-padatious >= 2.0.1a1folds the suffixed id onto the canonical form at registration time, so every match — and every dispatch — is canonical;ovos-workshop >= 9.3.2a1binds the handler to both spellings (improve common_qa/OCP conflicts #497), which survives either dispatch.Worth stating plainly:
ovos-coreis not the deciding component.IntentService._dispatch_matchforwardsmatch.match_typeverbatim, so the spelling on the wire is decided by the pipeline plugin's registry. That is why the "old core" pin below is really a pin on padatious.The pins, and the evidence
Both pins were chosen by behaviour, verified against source and tags, and are asserted at runtime by
test_pins_are_the_intended_vintage. A release that quietly changes either fails as a wrong-vintage error rather than turning the red cell green.ovos-workshop==9.3.1a2— newest release binding the suffixed topic only. The canonical binding arrived in27c8728(2026-07-26, #497), first released in9.3.2a1;9.3.1a2is the last tag without it. Verified by running the real skill against a real bus in each venv:A newer pin would hide the breakage outright.
ovos-padatious==2.0.0a1+ovos-core==2.5.5a2—_dealias_intent_namelanded in padatious2.0.1a1;2.0.0a1is the last tag without it, with core pinned to its contemporary release. The suite probes for the real symbol rather than a version string, so it keeps telling the truth if the fold ever moves.Where the fix has to land
The frozen container does not ship a frozen
ovos-bus-client. A workshop pin is a dependency floor, not a ceiling, so a rebuilt container resolves today's client —uvresolvingovos-workshop==9.3.1a2picks upovos-bus-client==2.7.2a1. The suite asserts this rather than assuming it, because the whole repair strategy depends on it.That is also why bus-client#271 puts its mirror on the receive side and keeps it off the wire: the fix must execute in the process that owns the stale binding, and only that process knows what it bound. The old skill's own
bus.on("….intent")fills anIntentAliasRegistry; when the canonical dispatch arrives, its client mirrors it locally onto the suffixed twin, and the handler runs with neither the skill nor the wire changing.Kill-switch gate
These cells gate the compat train in both directions:
old skill/new corestarts passing,strict=Trueturns that into a loud XPASS failure, and that failure is the signal to drop the marker and keep the guard permanently;new skill/old corered. That cell must not be deleted to make fr-fr/translate #500 green — it is the signal that the inbound direction (suffixed wire topic → canonical local listener) is still missing from the client;emit_legacyoff and asserts the handler stays silent. It passes today for the trivial reason that no mirror exists; once default VAD to "ovos-vad-plugin-webrtcvad" + setup/requirements cleanup #271 ships it becomes the real negative half of the pair. Any PR dropping the compat must flip these deliberately.CI
.github/workflows/backcompat_matrix.yml, separate frombuild_tests.ymlbecause that workflow installs a single environment and runs pytest in it.fail-fastis off so one broken combination cannot hide the other three, and each job uploads the versions its venvs actually resolved.Without
BACKCOMPAT_COMBOthe suite skips cleanly, so existing runs are unaffected.Channel cells — pinned by the live fleet, not just a boundary
Maintainer-requested extension: the boundary-pin cells above answer "where is
the line", but not "which side of the line is the fleet actually on". Four
more cells answer that, using the OVOS distro's own constraint files —
fetched fresh in
build_venvs.shat build time, never vendored, so the gatetracks the fleet:
constraints-stable.txt— the shipped stable imageconstraints-testing.txt— the testing channelstable-skill/dev-coreovos-workshop>=3.4.0,<3.5.0)xfail(strict=True)dev-skill/stable-coreovos-padatious>=1.4.2,<1.5.0)testing-skill/dev-coreovos-workshop>=7.0.6,<8.0.0)xfail(strict=True)dev-skill/testing-coreovos-padatious>=1.4.3,<2.0.0)As pinned today (2026-08-01), both channels float their
ovos-workshopand
ovos-padatiousfloors well below the 9.3.2a1 / 2.0.1a1 boundaries theboundary-pin cells mark, so
stable-skill/dev-coreandtesting-skill/dev-corehit the identical known gap asold-skill/new-coreand carry the same
xfail(strict=True)for the same reason. The other twoare passing controls, like
new-skill/old-core.This is the point of pinning against a live URL instead of vendoring a
copy: the day either channel's constraints file bumps
ovos-workshoppast9.3.2a1 (or
ovos-padatiouspast 2.0.1a1) without the receiving side alsomoving, the corresponding cell goes red as a loud XPASS at that exact
moment — the fleet moved, and this is the trace. Each job uploads the
fetched constraints file as an artifact alongside the resolved venv freeze,
so a red channel cell is traceable to exactly what was pinned that day.
One real finding surfaced building these: on the channel-constrained side,
ovos-bus-clientis capped by the same constraints file (testing pins it<2.0.0, so the skill venv resolves1.5.0), unlike the boundary cells whereonly
ovos-workshopis pinned and its dependency floor is left to resolvewhatever
ovos-bus-clientis current.test_old_container_resolves_a_current_bus_clientnow treats that as a fleet-inventory fact for channel combos rather than
asserting it — a real stable/testing-channel container will need a
ovos-bus-clientbump on top of a workshop bump to receive the#271fix,not just the workshop bump the boundary cells describe.
Both layers — boundary pins and channel pins — stay in the matrix
deliberately: boundary pins document where the behavior line is and fail as
wrong-vintage if a release moves it; channel pins verify what the fleet is
actually running today against that line.
Compat train
intent_topicshelpers (merged, released in1.6.0a1)FakeBustwin layer 1 needsTwo copies, on purpose
ovos-test-harness#16 carries the same matrix. Every relevant repo carries its own back-compat suite so a breakage is traceable to the repo that caused it, and the harness duplicates in parallel as the cross-repo home.
Verified locally
All four combinations, real venvs, real
ovos-messagebus, no mocks:Maintainer-approved. Layer 1 implemented by Claude (sonnet); layer 2 by Claude (opus). Orchestrated by Claude Fable.
🤖 Generated with Claude Code
Red CI diagnosis (ovoscope check). The single failing check is pre-existing on
dev, independent of this branch:dev's ownOvoscope End-to-End Testsworkflow fails the same way on unrelated recent pushes (runs 30673793131, 30672760305, 30094248519 — allfailure), with the same signature —test_activate.py::TestDeactivate::test_deactivate/test_activatetiming out waiting forovos.utterance.handled, plustest_intent_alias_backcompat.pysubfailures. This branch touches neither activation/deactivation dispatch nor the intent-alias backcompat path. Not fixed in-branch; tracked as a pre-existingdevissue.