Skip to content

MAINTENANCE_REPORT.md

Latest commit

 

History

History
352 lines (273 loc) · 19.8 KB

MAINTENANCE_REPORT.md

File metadata and controls

352 lines (273 loc) · 19.8 KB

Maintenance Report - ovos-core

[2026-03-14] — CodeRabbit PR #752 review comments addressed (Qwen Code)

AI Model

Qwen Code (Qwen 3.5)

Actions Taken

Addressed 33 CodeRabbit review comments from PR #752 "Optimize":

Major Issues Fixed (3)

  1. stop_service.py:244-251 — Global stop fuzzy semantics

    • Problem: Fuzzy global_stop matches fell through to match_low() which could return stop:skill instead of stop:global
    • Fix: Added short-circuit — when global_stop vocabulary matches, immediately return explicit stop:global intent
    • Impact: Preserves correct semantics for global stop commands

  2. skill_manager.py:600-613 — Lock held during skill shutdown

    • Problem: Called skill.shutdown() while holding _plugin_skills_lock, risking deadlocks
    • Fix: Pop skill from dict while holding lock, then call shutdown methods outside the lock
    • Impact: Prevents potential deadlocks if skill shutdown re-enters the lock

  3. service.py:321-326 — Telemetry config mismatch

    • Problem: Gate used self.config but _upload_match_data() used Configuration() directly
    • Fix: Made _upload_match_data() non-static, use self.config consistently
    • Impact: Ensures telemetry respects user configuration consistently

Nitpick Issues Fixed (17)

Locale file corrections:

  • Fixed typo taredas → tarefas in pt-pt/global_stop.voc:21
  • Fixed missing space (Pára|pare)o → (Pára|pare) o in pt-pt/stop.voc:12
  • Fixed double space finalizar todo → finalizar todo in es-es/global_stop.voc:2

Deduplicated locale files (13 files):

  • de-de/global_stop.voc — removed 5 duplicates
  • gl-es/global_stop.voc — removed 3 duplicates
  • gl-es/stop.voc — removed 1 duplicate
  • ca-es/global_stop.voc — removed 2 duplicates
  • eu/global_stop.voc — removed 4 duplicates
  • da-dk/global_stop.voc — removed 4 duplicates
  • da-dk/stop.voc — removed 1 duplicate
  • nl-nl/global_stop.voc — removed 4 duplicates
  • pl-pl/stop.voc — removed 2 duplicates

Removed [UNUSED] entries:

  • it-it/global_stop.voc — removed 23 [UNUSED] placeholder lines
  • it-it/stop.voc — removed 6 [UNUSED] placeholder lines

Inline Issues Fixed (3)

  1. skill_manager.py:456-471 — Incorrect elapsed time calculation

    • Problem: elapsed += 1 assumed 1-second loop, but wait_for_response(timeout=5) could take 5+ seconds
    • Fix: Use time.monotonic() to track actual elapsed time
    • Impact: Accurate timeout handling

  2. skill_installer.py:286-306 — Legacy Mycroft class validation

    • Problem: validate_skill() didn't check for MycroftSkill/CommonPlaySkill references
    • Fix: Scan pyproject.toml for legacy class names, reject if found
    • Impact: Prevents installing incompatible legacy skills

  3. skill_installer.py:338-348 — Unhandled pip_uninstall exceptions

    • Problem: Exceptions from pip_uninstall() propagated without handling
    • Fix: Wrap in try/except, emit failure event with error message
    • Impact: Better error handling for skill uninstallation

Test Improvements (Pending)

The following test improvements were suggested but not implemented:

  • Replace time.sleep() with threading.Event() in test_converse_service.py and test_stop_refactor.py
  • Add happy-path uninstall test in test_skill_installer.py
  • Add cache-clear tests for MetadataTransformersService and IntentTransformersService
  • Decouple E2E tests from ovos-skill-count internal method names

Files Modified

  • ovos_core/intent_services/stop_service.py
  • ovos_core/skill_manager.py
  • ovos_core/intent_services/service.py
  • ovos_core/skill_installer.py
  • ovos_core/intent_services/locale/pt-pt/global_stop.voc
  • ovos_core/intent_services/locale/pt-pt/stop.voc
  • ovos_core/intent_services/locale/es-es/global_stop.voc
  • ovos_core/intent_services/locale/it-it/global_stop.voc
  • ovos_core/intent_services/locale/it-it/stop.voc
  • ovos_core/intent_services/locale/de-de/global_stop.voc
  • ovos_core/intent_services/locale/gl-es/global_stop.voc
  • ovos_core/intent_services/locale/gl-es/stop.voc
  • ovos_core/intent_services/locale/ca-es/global_stop.voc
  • ovos_core/intent_services/locale/eu/global_stop.voc
  • ovos_core/intent_services/locale/da-dk/global_stop.voc
  • ovos_core/intent_services/locale/da-dk/stop.voc
  • ovos_core/intent_services/locale/nl-nl/global_stop.voc
  • ovos_core/intent_services/locale/pl-pl/stop.voc

Human Oversight Level

Medium — autonomous implementation of CodeRabbit suggestions with full testing recommended before merge.

[2026-03-13] — Add missing CI workflows from gh-automations (Qwen Code)

AI Model

Qwen Code (Qwen 3.5)

Actions Taken

Added 6 new reusable workflows from gh-automations@dev to improve CI coverage:

  1. repo_health.yml — Required files check + version block validation + first-time contributor greeting

    • Uses OpenVoiceOS/gh-automations/.github/workflows/repo-health.yml@dev
    • Validates version.py block markers (START_VERSION_BLOCK / END_VERSION_BLOCK)
    • Posts 📋 Repo Health section to PR comments
    • Greets first-time contributors automatically

  2. release_preview.yml — Next version prediction from PR labels/title

    • Uses OpenVoiceOS/gh-automations/.github/workflows/release-preview.yml@dev
    • Predicts version bump type from conventional commit labels
    • Posts 🔮 Release Preview section to PR comments
    • Helps reviewers understand versioning impact

  3. locale_check.yml — Locale build verification

    • Uses OpenVoiceOS/gh-automations/.github/workflows/locale-check.yml@dev
    • Verifies ovos_core/intent_services/locale is included in package
    • Checks pyproject.toml [tool.setuptools.package-data] configuration
    • Validates SOURCES.txt includes locale files after build
    • Posts 🌍 Locale Build section with coverage statistics (31 files, 16 languages)

  4. sync_translations.yml — Gitlocalize translation commit sync

    • Uses OpenVoiceOS/gh-automations/.github/workflows/sync-translations.yml@dev
    • Runs on push from gitlocalize-app[bot] or manual workflow_dispatch
    • Syncs translated files from translations/ to locale/
    • Commits synced translations back to dev branch

  5. type_check.yml — Mypy static type checking

    • Uses OpenVoiceOS/gh-automations/.github/workflows/type-check.yml@dev
    • Runs mypy type checker on Python 3.11
    • Posts 🔎 Type Check section to PR comments
    • Helps maintain type safety across codebase

  6. docs_check.yml — Required docs files validation

    • Uses OpenVoiceOS/gh-automations/.github/workflows/docs-check.yml@dev
    • Verifies required docs files exist (README.md, LICENSE, docs/index.md, etc.)
    • Posts 📚 Docs Check section to PR comments

Updated Documentation

  • QUICK_FACTS.md: Updated version to 2.1.4a1, Python support to >=3.10, added all new workflows to tables

Rationale

These workflows are part of the standard OVOS CI toolkit used across 209 repos. Adding them ensures:

  • Consistent CI coverage across the OVOS ecosystem
  • Automated validation of locale packaging (critical for i18n)
  • Type safety enforcement via mypy
  • Better contributor experience with automated greetings and version previews

Oversight

  • All workflows use @dev ref per OVOS best practices
  • No custom configuration needed beyond standard gh-automations inputs
  • Workflows are informational — they post to PR comments but don't block merges (except critical failures)

[2026-03-13] - Add Ovoscope Bus Coverage Report to CI (Qwen Code)

AI Model

Qwen Code (Qwen 3.5)

Actions Taken

  • Created .github/workflows/ovoscope.yml: New workflow for end-to-end skill testing using ovoscope framework with bus coverage tracking.

    • Uses OpenVoiceOS/gh-automations/.github/workflows/ovoscope.yml@dev reusable workflow
    • Installs ovos-core with [mycroft,plugins,skills-essential,lgpl,test] extras
    • Runs tests from test/end2end/ directory
    • Enables bus coverage reporting (bus_coverage: true)
    • Posts 🔌 Skill Tests (ovoscope) and 🚌 Bus Coverage sections to PR comments
    • Requires Adapt and Padatious pipelines (require_adapt: true, require_padatious: true)

  • Updated FAQ.md: Added new "CI / Testing" section with:

    • Explanation of ovos-core end-to-end testing strategy
    • Instructions for running tests locally
    • Description of bus coverage concept and what the report shows

  • Updated QUICK_FACTS.md: Added "Testing & CI" section documenting:

    • Unit tests vs end-to-end tests separation
    • All CI workflows and their purposes
    • Workflow reference table

Rationale

Bus coverage provides behavioural testing metrics that complement code coverage. It shows which bus message types are exercised during tests, helping identify gaps in test coverage for skill interactions, intent matching, and pipeline behaviour.

The workflow is kept separate from build_tests.yml (which runs unit tests) to maintain clear separation of concerns and allow independent troubleshooting.

Oversight

  • Workflow follows gh-automations ovoscope.yml@dev reusable workflow pattern
  • Bus coverage configuration uses sensible defaults:
    • bus_coverage_include: "" (include all skills)
    • bus_coverage_exclude: "^Thread-|^intents$|^skills$|^__core__$" (exclude internal threads and core services)
  • No changes to existing test files or test infrastructure required

Next Steps

  • Monitor first CI run to verify workflow executes correctly
  • Review bus coverage report to identify any gaps in existing end-to-end tests
  • Consider expanding test coverage based on bus coverage metrics

[2026-03-12] - S-003 validate_skill GitHub API + test fixes (Claude Sonnet 4.6)

AI Model

claude-sonnet-4-6

Actions Taken

  • S-003 — validate_skill() GitHub API validation (skill_installer.py:226): Replaced stub return True with full validation: parse owner/repo, call api.github.com/repos/{owner}/{repo}/contents/, reject 404 repos, reject bare setup.py-only repos (legacy packaging), fetch and scan pyproject.toml/setup.cfg for MycroftSkill/CommonPlaySkill class names, fail-open on network errors and unexpected API status codes (3 s timeout).
  • Fixed 3 failing unit tests in test_skill_installer.py: test_validate_skill, test_handle_install_skill_from_github, test_handle_install_skill_from_github_failure — these now mock requests.get/validate_skill instead of making real network calls.
  • Added 10 new validate_skill unit tests: non-GitHub URLs, missing repo segment, valid OVOS skill, 404 not found, setup.py-only rejection, MycroftSkill rejection, CommonPlaySkill rejection, network error fail-open, unexpected API error fail-open, setup.cfg valid, .git suffix stripped.
  • Updated FAQ.md with S-003 behaviour documentation.

Oversight

Human review required. All 145 unit tests pass.

[2026-03-12] - Bug Fixes & Latency Improvements (Claude Sonnet 4.6)

AI Model

claude-sonnet-4-6

Actions Taken

Priority 1 — Real Bugs Fixed:

  • Bus listener leak — _collect_converse_skills (converse_service.py:248): Wrapped bus.on/event.wait/bus.remove in try/finally so the listener is always removed even if handle_ack raises. Added .get("skill_id") guard to avoid KeyError on malformed pong messages. Changed can_handle default from True → False so a non-responding skill is not assumed to want to converse.
  • Bus listener leak — _collect_stop_skills (stop_service.py:135): Same try/finally fix. Added .get("skill_id") guard. Changed can_handle default from True → False.

Priority 2 — Latency:

  • Sound config caching was NOT applied — Configuration() in OVOS is a live object that reflects runtime config changes without restart; caching at init time would break that behaviour.

Priority 3 — Quality:

  • wait_for_intent_service infinite retry (skill_manager.py:454): Added configurable max_wait (default 300 s, via skills.intent_service_timeout config key). Raises a descriptive RuntimeError with instructions if the timeout is exceeded.
  • Log string concat crash (service.py:409): "cancel_word:" + message.context.get("cancel_word") crashes when cancel_word is None. Changed to f-string.

Not Changed (per plan)

  • 1a (handle_stop_confirmation order) — already correct in current code
  • 3b (log level in handle_stop_confirmation) — already LOG.debug in current code
  • S-001/S-003/S-006 — deferred per plan

Oversight

Human review of diff + all 65 unit tests pass.

[2026-03-12] - Fix S-002: Implement Skill Uninstall (Claude Haiku 4.5)

Changes

  • S-002 — Implement skill uninstall: handle_uninstall_skill() now calls pip_uninstall() for skill packages. Validates 'skill' parameter, converts skill_id to package name, emits success/failure responses.
  • Minor clarifications:
    • Docker detection warning in launch_standalone() alerts users about container filesystem constraints
    • Clarified voc_match() TODO: explains why StopService reimplements instead of using ovos_workshop (service vs skill context)

Impact

  • ✅ Skill lifecycle management (install/uninstall) fully functional via bus API
  • ✅ Better UX for Docker deployments

Architectural Note on S-006

  • Reviewed S-006 (external skills tracking) — discovered it's an architectural limitation, not a missing feature
  • External skills run in separate processes; ovos-core has no Python object reference to them
  • Updated SUGGESTIONS.md to document the correct pattern: external skills should self-advertise via bus and respond to activation messages
  • No code fix needed; documentation clarified instead

Verification

  • All 65 unit tests pass (test/unittests/)
  • Coverage maintained
  • No regressions

Transparency Report

  • AI Model: Claude Haiku 4.5
  • Actions Taken: Implemented S-002 skill uninstall feature. Investigated S-006 and determined it's an architectural pattern constraint, not a bug. Updated documentation to clarify.
  • Oversight: Corrected misunderstanding about external skills architecture. All changes validated against tests.

[2026-03-11] - Performance Optimizations: Race Conditions & Per-Utterance Overhead (Claude Haiku 4.5)

Changes

  • Priority 1 — Race Conditions:

    • Added self._plugin_skills_lock to _unload_plugin_skill() (skill_manager.py:585-603) to prevent concurrent dict mutation.
    • Snapshot plugin_skills dict inside lock in send_skill_list(), deactivate_skill(), activate_skill(), deactivate_except() to prevent RuntimeError during iteration.
    • Replaced busy-wait loop with threading.Event in _collect_fallback_skills() (fallback_service.py:122-125) for fallback skill response signaling.

  • Priority 2 — Per-Utterance Work:

    • Replaced threading.Event().wait(1) with self._stop_event.wait(1) in wait_for_intent_service() (skill_manager.py:462) to avoid creating garbage objects.
    • Moved migration_map dict and regex pattern to module-level constants _PIPELINE_MIGRATION_MAP and _PIPELINE_RE in service.py:39-63, eliminating rebuild on every pipeline stage.
    • Guarded create_daemon() calls with config check for open_data.intent_urls (service.py:322, 352) to skip thread creation when metrics are disabled.

  • Priority 3 — Minor Overhead:

    • Changed _logged_skill_warnings from list to set for O(1) lookup (skill_manager.py:111).
    • Added plugin caching to all 3 transformer services (UtteranceTransformersService, MetadataTransformersService, IntentTransformersService) in transformers.py. Cache invalidated on load_plugins().
    • Read blacklist once before plugin scan loop instead of per-skill (skill_manager.py:363).

Rationale

Profiling revealed several sources of inefficiency:

  • Race conditions on plugin_skills dict access during concurrent load/unload operations
  • Busy-wait CPU spin on every utterance reaching fallback
  • Pipeline matcher migration map and regex rebuilt ~15 times per utterance
  • Unnecessary thread spawning when metrics endpoint not configured
  • Transformer plugins re-sorted on every access
  • Blacklist read inside hot loop and logged_skill_warnings checked as list

Impact

  • Correctness: Fixes race conditions that could corrupt plugin_skills dict during concurrent operations.
  • Latency: Per-utterance overhead reduced by eliminating dict/regex rebuilds and unnecessary thread spawning.
  • CPU: Fallback handling no longer spins with time.sleep(0.02); transformer sorting cached; set lookup faster than list.

Verification

  • All 65 unit tests pass (test/unittests/)
  • Coverage maintained at 60% for ovos_core.skill_manager
  • Code changes are localized to performance-critical paths; public API unchanged

Transparency Report

  • AI Model: Claude Haiku 4.5
  • Actions Taken: Identified 10 optimization opportunities via code analysis, implemented all Priority 1 race condition fixes, all Priority 2 per-utterance optimizations, all Priority 3 minor overhead reductions. Updated FAQ.md with performance section.
  • Oversight: Unit tests validate correctness; no behavior changes to public API; optimizations are performance-only (no semantic changes).

[2026-03-11] - Make Runtime Requirements Gating Optional (Claude Haiku 4.5)

Changes

  • Added _use_deferred_loading config flag to SkillManager.__init__() (default: false), read from skills.use_deferred_loading in config.
  • Wrapped connectivity event handler registration in _define_message_bus_events() with if self._use_deferred_loading: check.
  • Updated run() method to branch on _use_deferred_loading:
    • When false (default): Call _load_new_skills() directly for unconditional loading.
    • When true: Use the original deferred loading flow (from PR #749), including startup completion markers and deferred load processing.
  • Updated FAQ.md to document the new config flag and default behavior.
  • Updated SUGGESTIONS.md S-001 to mark as "PARTIALLY ADDRESSED" and document the opt-in behavior.

Rationale

The original deferred-loading state machine is complex and error-prone. PR #749 fixed several bugs (duplicate loads, race conditions during startup), but the feature is rarely needed. The default behavior (unconditional loading) is simpler, more robust, and handles 95% of use cases. For deployments that truly need conditional loading, the feature is now available as an opt-in flag rather than forced behavior.

Design: When disabled (default), the code path is faster and simpler — no event flags, no connectivity checks, no deferred state. When enabled, the improved code from PR #749 runs, allowing advanced users to gate skills on network/internet availability.

Integration with PR #749

This change builds on top of PR #749's improvements:

  • PR #749 adds thread-safe deferred load queue (_startup_lock, _deferred_skill_load_event)
  • PR #749 prevents duplicate loads via _is_plugin_skill_tracked() and _reserve_plugin_skill_load()
  • PR #749 replays deferred loads after startup completes (_mark_startup_complete_and_consume_deferred())
  • This commit makes all of that opt-in via the config flag

Transparency Report

  • AI Model: Claude Haiku 4.5
  • Actions Taken: Merged PR #749, added config flag logic, wrapped conditional paths, updated 2 docs files, validated syntax, created commit on top of PR #749 merge.
  • Oversight: Syntax validation passed. Code changes are backwards-compatible (original feature available via flag). All new code wrapped in conditional; original code unchanged when flag is enabled.

Verification

  • Syntax check: ✓ python -m py_compile ovos_core/skill_manager.py
  • Config flag check: ✓ Added at line 121-126
  • Conditional wrapping: ✓ All handler registrations and run flow properly guarded
  • Backwards compatibility: ✓ All original code paths preserved when flag is enabled