DO NOT MERGE: drop all backwards compatibility (BREAK train reference) - #912
DO NOT MERGE: drop all backwards compatibility (BREAK train reference)#912JarbasAl wants to merge 1 commit into
Conversation
…ain (DO NOT MERGE) Reference commit for the flag-day break: removes the pre-STOP-1 _LegacyStopBridge (mycroft.stop/<skill_id>.stop re-emission, stop_service_legacy.py, wired from StopService), drops _PIPELINE_MIGRATION_MAP (silent acceptance of legacy short pipeline-matcher ids like "adapt_high" in intents.pipeline/blacklisted_pipelines; the spec-correct full plugin ids like "ovos-adapt-pipeline-plugin-high" are the only accepted form now), and removes SkillManager.stop() (bare backwards-compat alias for shutdown()). Tests pinning only the removed behavior are dropped with it; tests using the legacy ids as arbitrary sample strings are updated to spec ids. 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 |
I've completed my sweep! Here's the situation. 🧹I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthI've performed a digital acupuncture on the codebase. 📍 ✅ All required files present. Latest Version: ✅ 🌍 Locale BuildI've finished the digital walk-through of your PR. 🚶♂️ ✅ Locale properly configured (65 files, 18 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 32 locale files included in package 🔒 Security (pip-audit)Our digital defenses have been updated. 🛡️ ✅ No known vulnerabilities found (113 packages scanned). 📡 Channel Compat — stableHere's the report you've been waiting for. 📁 🚧 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 📚 DocsI've double-checked the data for any anomalies. 🔍 ✅ All required documentation files present. ✅ 🏷️ Release PreviewThe release preview is now complete. 🏁 Current:
🚀 Release Channel Compatibility Predicted next version:
📊 CoverageCoverage report incoming! Every line counts. 🎯 Files below 80% coverage (9 files)
Full report: download the 🔨 Build TestsChecking if all the bolts are tightened. 🔩 ✅ All versions pass
🔌 Plugin DetectionI've checked the plugin's 'documentation' links. 📖 ❌ Plugin Status: ERRORS (1) Plugin Info:
OPM Detection:
Entry Point Validation:
⊘ No Issues:
⚖️ License CheckVerifying the SPDX identifiers for correctness. 🆔 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔌 Skill Tests (ovoscope)I've simulated a thousand conversations with your skill. 🗣️ ❌ 11/35 passed, 5 failed ❌ **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 CoverageEnsuring the skill's emissions match our expectations. 📤 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
1 participant
DO NOT MERGE — reference for the breaking release train; the owner merges when the time comes.
This is a scoped slice of the total backwards-compat drop, distinct from the held flag-day PR #837: rather than describing the full break, it shows the concrete diff for three self-contained compat surfaces so their removed-line counts are visible ahead of time. It does not duplicate #837's description or scope; treat the two as complementary reference points for the same release.
The pre-STOP-1
_LegacyStopBridge(ovos_core/intent_services/stop_service_legacy.py) re-emittedmycroft.stop/<skill_id>.stopand the pre-specstop:global/stop:skillobserver topics on top ofStopService's spec-correct<skill_id>:stop/<pipeline_id>:global_stopdispatch, purely for deployments or skills that never adopted the spec topics. It is wired out ofStopService.__init__/shutdownand the module and its dedicated end-to-end/unit tests are deleted with it._PIPELINE_MIGRATION_MAPinovos_core/intent_services/service.pysilently translated legacy short pipeline-matcher ids (adapt_high,padatious_high,ocp_legacy, and so on) found inintents.pipeline/session.blacklisted_pipelinesinto today's full plugin ids before resolving a matcher or attributing a dispatch. Emission was always spec-correct (pipeline_idonovos.intent.matchedalready used the full plugin id); only the acceptance of the old short spelling on the way in is dropped. A config still carrying the old spelling now fails to resolve a matcher rather than being silently normalized.SkillManager.stop()was a one-line alias forshutdown()kept for callers that never moved to the current name; it is dropped along with its dedicated unit test.Tests that existed only to pin the removed acceptance (the
_LegacyStopBridgebehavior tests, the migration-map resolution test, the blacklist-warning test keyed specifically on a legacy short id) are deleted. Tests that merely used a legacy short id as an arbitrary sample pipeline-id string are updated to a spec-correct id so they keep testing the surrounding logic.Removal table
_LegacyStopBridge/stop_service_legacy.py+ wiringStopService's own spec dispatch (<skill_id>:stop,<pipeline_id>:global_stop,ovos.stop)test_stop_legacy_e2e.py+ legacy-bridge unit tests_PIPELINE_MIGRATION_MAPlegacy short-id acceptanceovos-adapt-pipeline-plugin-high)SkillManager.stop()aliasSkillManager.shutdown()Diffstat
Not removed — needs a call
dispatcher.py's consumption ofmycroft.skill.handler.complete/.erroras the framework done-signal, andskill_manager.py'smycroft.network.connected/mycroft.internet.connected/mycroft.gui.available/mycroft.skills.*topics.ovos-spec-tools'NamespaceTranslator.is_migrated(...)returnsFalsefor these pairs (confirmed against the installed translator), so they are not a translator-bridged legacy alias of an existing spec topic — dropping them would be a protocol change to what ovos-workshop skills and ovos-audio actually emit today, not a compat-shim removal, and is out of scope here.converse_service.py's dual-emit of the broadcast and per-skill legacy converse ping/pong (skill.converse.pong) — this keeps currently-shipping skills in the converse contest and is load-bearing today, not a droppable shim.skill_installer.py's rejection of baresetup.py-only ("legacy Mycroft packaging") skill repos — this is validation logic for what it accepts, not a compat surface to remove.Test evidence
Targeted suites for the touched files (
test_stop_service.py,test_skill_manager.py,test_intent_service_extended.py): 156 passed, 9 subtests passed.Full suite on this branch: 494 passed, 6 xfailed, 47 subtests passed, 11 failed. All 11 failures reproduce identically against unmodified
dev(verified in a separate clone) — 10 are pre-existingovoscopemodel-load/timing flakiness intest_padatious.py,test_adapt.py,test_converse.py,test_intent_pipeline.py; the 11th,test_stop_spec_e2e.py::TestTargetedStopSpec::test_targeted_stop_dispatch_shape, is the area this PR touches directly and passes both standalone and in a subsequent full end2end-only re-run (which reproduced only the same 10 pre-existing flaky failures). None of the 11 are introduced by this change.