fix: scheduler delegation test reads skill_id, not owner - #608
Conversation
…test ovos-bus-client 2.11.14a1 renamed the SCHEDULER-1 record identity key from owner to skill_id in the live get()/list() view a consumer sees; the from_stored migration shim in schedules.py only covers the on-disk read boundary, not this view. ovos_workshop has no "owner" reader at all, so the old assertion was asserting a key the current library never produces. Bisected the published wheels on a clean origin/dev worktree: 2.11.12a1, 2.11.13a1, 2.11.13a3 and 2.11.13a5 give 44 passed in this file; 2.11.14a1 and 2.11.14a2 give 1 failed, 43 passed with KeyError owner at the assertion. After this change: 44 passed in the file and 702 passed with 18 subtests across test/unittests against ovos-bus-client 2.11.14a2. 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 |
Hello there! Your PR checks are ready for review. ✨I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthI've checked the repo's strength (aka test suite robustness). 🏋️♂️ ✅ All required files present. Latest Version: ✅ ⚖️ License CheckI've checked the licenses of all dev-dependencies too. 🛠️ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 📡 Channel Compat — stableI've checked the vitals of this contribution. 🩺 🚧 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 🔒 Security (pip-audit)Ensuring our digital fortress remains impenetrable. 🏰 ✅ No known vulnerabilities found (72 packages scanned). 📡 Channel Compat — testingGeneric report status: Complete and verified. ✅ 🚧 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-testing.txt 🔍 LintEvaluating the overall quality of your PR. ✨ ❌ ruff: issues found — see job log 🔨 Build TestsFrom source to binary, let's see how it holds up. 🧱 ✅ All versions pass
Providing clarity through automated analysis 🔍 |
OVOS-STOP-1 §4.2 makes `ovos.stop.ping` the stop cascade's poll topic and treats a handler that does not subscribe to it as `can_handle: false` for that round. OVOSSkill subscribed only to the pre-spec per-skill `<skill_id>.stop.ping`, so every skill's stoppability rested on the compatibility twin ovos-core still emits alongside the broadcast; once that twin goes, generic stop falls back to the recency rule for the whole fleet. OVOSSkill now subscribes to both pings and answers on `ovos.stop.pong` with the §4.2 payload. A core emitting both in one round draws exactly one pong: STOP-1 §4.2 binds the poll to PIPELINE-1 §4.5, which keys round state by the `session_id` of `context.session` and by `context.utterance_id` (§9.1.1) — both propagate to either ping by reply derivation — and rules that the first valid pong per candidate wins. A pre-spec core that stamps no `utterance_id` names no round, so its pings are answered one for one as before. Fail-before: the broadcast test fails on dev (0 pongs, nothing subscribes); with the subscription but no dedupe the single-pong test fails 2 != 1; a session-only round key fails two rounds in one session, 1 != 2. Suite: 708 passed, 1 pre-existing failure in test_scheduler_delegation (KeyError 'owner', PR #608). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
OVOS-STOP-1 §4.2 makes `ovos.stop.ping` the stop cascade's poll topic and treats a handler that does not subscribe to it as `can_handle: false` for that round. OVOSSkill subscribed only to the pre-spec per-skill `<skill_id>.stop.ping`, so every skill's stoppability rested on the compatibility twin ovos-core still emits alongside the broadcast; once that twin goes, generic stop falls back to the recency rule for the whole fleet. OVOSSkill now subscribes to both pings and answers on `ovos.stop.pong` with the §4.2 payload. A core emitting both in one round draws exactly one pong: STOP-1 §4.2 binds the poll to PIPELINE-1 §4.5, which keys round state by the `session_id` of `context.session` and by `context.utterance_id` (§9.1.1) — both propagate to either ping by reply derivation — and rules that the first valid pong per candidate wins. A pre-spec core that stamps no `utterance_id` names no round, so its pings are answered one for one as before. Fail-before: the broadcast test fails on dev (0 pongs, nothing subscribes); with the subscription but no dedupe the single-pong test fails 2 != 1; a session-only round key fails two rounds in one session, 1 != 2. Suite: 708 passed, 1 pre-existing failure in test_scheduler_delegation (KeyError 'owner', PR #608). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
OVOS-STOP-1 §4.2 makes `ovos.stop.ping` the stop cascade's poll topic and treats a handler that does not subscribe to it as `can_handle: false` for that round. OVOSSkill subscribed only to the pre-spec per-skill `<skill_id>.stop.ping`, so every skill's stoppability rested on the compatibility twin ovos-core still emits alongside the broadcast; once that twin goes, generic stop falls back to the recency rule for the whole fleet. OVOSSkill now subscribes to both pings and answers on `ovos.stop.pong` with the §4.2 payload. A core emitting both in one round draws exactly one pong: STOP-1 §4.2 binds the poll to PIPELINE-1 §4.5, which keys round state by the `session_id` of `context.session` and by `context.utterance_id` (§9.1.1) — both propagate to either ping by reply derivation — and rules that the first valid pong per candidate wins. A pre-spec core that stamps no `utterance_id` names no round, so its pings are answered one for one as before. Fail-before: the broadcast test fails on dev (0 pongs, nothing subscribes); with the subscription but no dedupe the single-pong test fails 2 != 1; a session-only round key fails two rounds in one session, 1 != 2. Suite: 708 passed, 1 pre-existing failure in test_scheduler_delegation (KeyError 'owner', PR #608). Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
ovos-bus-client 2.11.14a1 renamed the SCHEDULER-1 record identity key from
ownertoskill_id. The rename covers the on-disk read boundary inschedules.py'sfrom_storedmigration shim, but not the liveget()/list()view a consumer actually sees.ovos_workshophas no"owner"reader anywhere in the package, sotest_a_delay_in_seconds_becomes_a_relative_timingwas asserting a key the current library never produces, and dev has been red on this single line since the bump.The fix is the test assertion, not the library:
record["owner"]becomesrecord["skill_id"]at test/unittests/test_scheduler_delegation.py line 196.Bisected the published ovos-bus-client wheels on a clean origin/dev worktree: 2.11.12a1, 2.11.13a1, 2.11.13a3 and 2.11.13a5 all give 44 passed in this file; 2.11.14a1 and 2.11.14a2 give 1 failed, 43 passed with
KeyError: ownerat the assertion. With the old key in place against 2.11.13a5, the fixed assertion fails as expected (proving the test is real); against 2.11.14a2 the file passes 44/44, and the fulltest/unittestssuite passes 702, with 18 subtests, and zero failures.This unblocks ovos-workshop#605, whose five red build_tests legs are this same single failure rather than anything in that PR's own diff.