Conversation
|
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 |
Automated check summary ready. 📊I've aggregated the results of the automated checks for this PR below. 🔍 LintI've tidied up the results for you. 🧹 ❌ ruff: issues found — see job log 🏷️ Release PreviewThe roadmap for the future just got clearer. 🗺️ Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
⚖️ License CheckEnsuring our project is well-protected legally. 🛡️ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 📋 Repo HealthScanning for any signs of 'dependency' parasites. 🐛 ✅ All required files present. Latest Version: ✅ 🔒 Security (pip-audit)Security check! Are we safe from vulnerabilities? 🛡️ ✅ No known vulnerabilities found (77 packages scanned). 📊 CoverageTesting the limits! Here's the coverage breakdown. 📏 Files below 80% coverage (15 files)
Full report: download the 🔨 Build TestsStructural analysis of your contribution is complete. 🔬
❌ 3.10: Install OK, tests failed Automatically generated, personally appreciated 💖 |
558cbf8 to
946b949
Compare
ovoscope's core dependency was ovos-core[lgpl,plugins], so every harness install pulled in ovos_padatious regardless of what a suite actually asked for — a padacioso/m2v-only cell saw an extra mycroft.skills.trained bus message it never subscribed to (Padatious registers and trains synchronously in the background). Padatious is being archived and must not be implied by the base install. The dependency is now plain ovos-core>=2.0.4a2. ovos-workshop (an ovos-core dependency) pins padacioso unconditionally, so a bare install still ships one pipeline matcher. Adapt and Model2Vec move into ovoscope's own `engines` extra (used by the multi-engine golden runner and the live pipeline harness); a new `padatious` extra forwards to ovos-core[plugins] for suites that still need the legacy matcher. Five tests that exercised the full padatious/padacioso/nebulento gating set unconditionally are now marked `pytest.mark.padatious` (registered in pyproject) instead of skipping when the extra is absent — coverage that silently disappears on a missing dependency is itself a bug. CI runs both lanes on every PR: `build_tests` installs `.[test]` and runs `-m "not padatious"`; a new `padatious_cell` job (Python 3.11 only) installs `.[test,padatious]` and runs `-m padatious`. One tiny test in each lane asserts ovos-padatious's importability the way that lane expects, so a wrong extras list fails loudly instead of quietly changing the count. Fail-before: reverting only the pyproject.toml dependency line and reinstalling into a fresh venv pulls in ovos-padatious==2.1.1a1; with the fix, the same install raises PackageNotFoundError for ovos-padatious. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
946b949 to
d722ed1
Compare
Every ovoscope install used to pull in
ovos-core[lgpl,plugins], which pulls inovos_padatiouseven when a suite only wants the padacioso or Model2Vec pipelines. Padatious registers and trains synchronously in the background, so a padacioso-only test cell was seeing an extramycroft.skills.trainedbus message it never asked for (see OpenVoiceOS/ovos-skill-hello-world#125). Padatious is archived and should not ride along on a bare install.The dependency is now plain
ovos-core>=2.0.4a2.ovos-workshop, one of ovos-core's own dependencies, pinspadaciosounconditionally, so a bare install still ships that one pipeline matcher — nothing else.enginesovoscope.goldenmulti-engine runner, the live pipeline harness testspadatiousovos-core[plugins](Padatious)test/devenginesCoverage that silently disappears on a missing optional dependency is a bug in its own right, so the five tests that assumed the full padatious/padacioso/nebulento set was always installed are marked
pytest.mark.padatious(registered inpyproject.toml) instead of skipping. CI now runs two lanes on every PR:build_tests(Python 3.10–3.14) installs.[test]and runs-m "not padatious"; a newpadatious_celljob (Python 3.11) installs.[test,padatious]and runs-m padatious. One tiny test in each lane assertsovos-padatious's importability the way that lane expects (present in the padatious cell, absent in the base lane), so a wrong extras list fails loudly instead of quietly changing the pass count. Locally: base lane 681 passed / 27 skipped / 6 deselected; padatious lane 6 passed / 708 deselected.I checked every consumer in the OVOS org that installs
ovoscope[...]: they only use themediaandttsextras, neither of which touches padatious, adapt, or[plugins], so this change does not affect them.Fail-before: in a fresh venv, reverting only the
pyproject.tomldependency line and reinstalling pulls inovos-padatious==2.1.1a1(and uv warns thatovos-core's current PyPI release no longer even has a[lgpl]extra); with the fix the same install raisesPackageNotFoundErrorforovos-padatious,import ovoscopestill works, and the padacioso OPM pipeline entry point is present viaimportlib.metadata.Downstream integration: I installed this checkout editable over a fresh clone of
ovos-skill-hello-world'stest/drop-padatiousbranch (with theovoscopeline stripped from its e2e requirements) and ran its padacioso-only cell (TestNoAdaptPipeline): both tests pass, and a manual message capture on the same scenario shows nomycroft.skills.trainedmessage at all.Unrelated finding:
build_tests (3.10)failed three times ontest_media.py::TestOCPHarnessNamespaceBridging::test_no_bridging_isolates_spec_from_legacy(PlayerState.STOPPEDinstead ofPLAYING). This repo does not use pytest-randomly (no such plugin is installed or listed in the CI log), so ordering is deterministic, not seed-based. I reproduced it locally on a Python 3.10 venv by running the same ~456-test prefix that precedes it in collection order, but bisecting that prefix in half — and further down to just the real-MiniCroft-booting files — never reproduces it alone; only the full run does. That points to cumulative CPU/thread load exposing a fragiletime.sleep(0.05)-then-assert wait in the OCP harness test itself on the slower Python 3.10 interpreter, not a single discrete test leaking state. I did not find a specific leaking test to fix, so I have not opened a follow-up PR guessing at one; this is unrelated to the padatious/extras change in this PR either way.