fix: drop the tolerant playlist patch from the media harness - #146
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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 automated sentinels have completed their watch. 💂♂️I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthI've checked the repo's reflexes (aka build speed). ⚡ ✅ All required files present. Latest Version: ✅ 🏷️ Release PreviewA look ahead at the next milestone. 🚩 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
⚖️ License CheckThe license report is filed and ready for review. 📁 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintEverything looks good so far! ✅ ❌ ruff: issues found — see job log 📊 CoverageLet's see if we've left any dark corners in the test suite. 🔦 ❌ 59.0% total coverage Files below 80% coverage (15 files)
Full report: download the 🔨 Build TestsVerifying the structural soundness of your build. 🏗️ ✅ All versions pass
🔒 Security (pip-audit)Our digital defenses have been updated. 🛡️ ✅ No known vulnerabilities found (79 packages scanned). An automated hug for your code 🤗 |
5954bd9 to
9e58bcd
Compare
_TolerantPlaylist rebound ovos_media.player.Playlist to hide a bare-string constructor arg quirk. Against current ovos-media (>=2.0.0a9) the player's internal queue is its own PlayQueue, so the patch's only remaining effect is making a genuine ovos_utils Playlist fail the module-global isinstance check in set_now_playing. Delete the shim and bump the media/dev extras' ovos-media floor to 2.0.0a9 (the pre-restructure 0.0.2a3 pin was stale and never exercised by this harness code). The same restructure dropped BaseMediaService's validate_source parameter — session-source gating moved to the player's bus edge — so the harness's real- backend injection path (OCPPlayerHarness.__enter__, ovoscope/media.py) drops the stale kwarg too; nothing in the harness read AudioService.validate_source, so there was no behavior to replace. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9e58bcd to
60f95c9
Compare
OCPPlayerHarness.__enter__patchedovos_media.player.Playlistwith a local_TolerantPlaylistsubclass that dropped bare-string positional args before calling the realPlaylist.__init__. The comment explained the original intent:Playlist("Search Results")used to try to add the title string as a media entry, and the shim avoided that while still satisfyingisinstancechecks inplayer.py.Against the currently published ovos-media (2.0.0a9, matching PyPI's latest release and the same restructure the defect report describes), the player's internal queue is its own
PlayQueueobject, built and mutated independently of the module-globalPlaylistname.Playlist(...)is never called byplayer.pyto construct the queue, so the patch's only live effect left is rebinding the name thatset_now_playingchecks withisinstance(track, (MediaEntry, Playlist)). That means a caller who hands the harness a genuineovos_utils.ocp.Playlistgets rejected withValueError: Expected MediaEntry, but got: ..., because the object is an instance of the real class, not the harness's_TolerantPlaylistsubclass sitting in the patched module slot. The shim had inverted from "compatibility helper" to "active mis-binding."I deleted
_TolerantPlaylistand itspatch()call outright rather than making it conditional on the installed ovos-media version.ovoscope'smedia/devextras pinnedovos-media>=0.0.2a3, a floor from before the 1.0/2.0 restructure that the harness code already assumes elsewhere (PlayQueue, theGUIInterface-presence check). That old pin was never actually exercised — nothing in the harness would work against pre-restructure ovos-media regardless of this patch — so I bumped both floors to2.0.0a9, the version this repo genuinely supports, which makes "installed floor is already >=2.0.0a9" true and plain deletion correct.I added
TestOCPHarnessRealPlaylistIsinstance::test_real_playlist_passes_isinstance_in_set_now_playing, which drives a realPlaylistthroughplayer.set_now_playing()inside the harness and asserts it lands correctly. I red-proved it by temporarily reinstating the old patch: the test fails with theValueErrordescribed above, and passes again once the patch is gone.Full
test/suite: before the change, 534 passed, 12 failed, 111 skipped; after, 535 passed (the one new test), 12 failed, 111 skipped — identical failure set both times. All 12 are pre-existing and unrelated to this patch: three are aBaseMediaService.__init__() got an unexpected keyword argument 'validate_source'mismatch between the harness's real-AudioServiceinjection path and the installed ovos-media's constructor signature, and the other nine aretest_listener_stream.py/test_audit_round1.pyfailures around microphone-plugin loading and dinkum-listener teardown, unrelated to media/Playlist. Notest_tts_intelligibilityordering flake appeared in either run.