Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Python versions from 3.14 to 3.11 across multiple GitHub Actions workflows, removed the dev-to-master workflow, refactored STT plugin model configuration handling to be dynamic rather than hardcoded, updated plugin entry points from Mycroft to OpenVoiceOS standards, and widened the ovos-plugin-manager dependency constraint. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release_workflow.yml (1)
10-19: Add merge guard condition to prevent unintended alpha releases.The
publish_alphajob lacks anifcondition to check whether the pull request was merged. This means it will execute for allpull_requestclosed events, including those that were closed without merging.This could result in alpha releases being published for rejected or abandoned PRs. The
notifyjob on line 22 correctly guards on merge status, creating an inconsistency.🔒 Proposed fix to add merge guard
jobs: publish_alpha: + if: github.event_name == 'workflow_dispatch' || github.event.pull_request.merged == true uses: TigreGotico/gh-automations/.github/workflows/publish-alpha.yml@master secrets: inherit with:
🤖 Fix all issues with AI agents
In @.github/workflows/unit_tests.yml:
- Line 38: Add a python_requires spec to setup() in setup.py and update docs:
modify the setup() call to include python_requires=">=3.10" so pip refuses
installs on <3.10; update CHANGELOG.md to record that support for Python 3.8 and
3.9 was dropped (add an explicit entry under unreleased/release notes); and
update README.md to state the minimum required Python version (3.10+) in the
installation/requirements section.
In @ovos_stt_plugin_fasterwhisper/__init__.py:
- Around line 128-132: The default model name is incorrect: change the default
assigned to model in the init (currently "whisper-large-v3-turbo") to the
library-expected "large-v3-turbo" so FasterWhisperSTT.MODELS validation passes;
update the line that sets model (and any related log messages that reference the
default name) so the code uses "large-v3-turbo" when self.config.get("model") is
falsy and preserves the existing fallback/log flow.
In @setup.py:
- Around line 74-76: The entry_points mapping uses strings instead of lists so
setuptools/plugin discovery will fail; update the entry_points dict so each
group value is a list containing the respective constant (wrap
PLUGIN_ENTRY_POINT, CONFIG_ENTRY_POINT and LANG_PLUGIN_ENTRY_POINT each in a
list) while keeping the group keys ('opm.stt', 'opm.stt.config',
'opm.transformer.audio') unchanged.
🧹 Nitpick comments (1)
ovos_stt_plugin_fasterwhisper/__init__.py (1)
213-224: Consider model filtering or differentiated priorities.The dynamic config generation is a good improvement for maintainability. However, generating configurations for all models in
MODELS(which includes tiny, small, medium, large variants, and distilled versions) might create a very large config dictionary with many options.Consider whether:
- Some models should be filtered out (e.g.,
.envariants if multilingual support is a priority)- Different models should have different priority values based on quality/size trade-offs
- The display names could include more context (e.g., model size/speed characteristics)
💡 Example: Differentiated priorities based on model characteristics
+# Map model names to priorities (higher = better quality, lower = faster) +MODEL_PRIORITIES = { + "large-v3-turbo": 70, + "large-v3": 65, + "turbo": 60, + "medium": 55, + "small": 50, + "base": 45, + "tiny": 40, +} + FasterWhisperSTTConfig = { lang: [{ "model": model, "lang": lang, "meta": { - "priority": 50, + "priority": MODEL_PRIORITIES.get(model, 50), "display_name": f"FasterWhisper ({model})", "offline": True, }, } for model in FasterWhisperSTT.MODELS] for lang in FasterWhisperSTT.available_languages }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/build_tests.yml.github/workflows/dev2master.yml.github/workflows/license_tests.yml.github/workflows/publish_stable.yml.github/workflows/release_workflow.yml.github/workflows/unit_tests.ymlovos_stt_plugin_fasterwhisper/__init__.pyrequirements/requirements.txtsetup.py
💤 Files with no reviewable changes (1)
- .github/workflows/dev2master.yml
🧰 Additional context used
🪛 GitHub Actions: Run UnitTests
ovos_stt_plugin_fasterwhisper/__init__.py
[error] 143-143: ValueError: Invalid model size 'whisper-large-v3-turbo' when initializing WhisperModel. Expected one of: tiny.en, tiny, base.en, base, small.en, small, medium.en, medium, large-v1, large-v2, large-v3, large, distil-large-v2, distil-medium.en, distil-small.en, distil-large-v3, distil-large-v3.5, large-v3-turbo, turbo
🔇 Additional comments (9)
ovos_stt_plugin_fasterwhisper/__init__.py (2)
210-210: LGTM!The change to use
cls.LANGUAGES.keys()directly is cleaner and more maintainable than the previous implementation.
239-239: No action needed. Thejfk.wavfile exists at the repository root and the relative path correctly references it. The example code will function as intended..github/workflows/build_tests.yml (1)
16-16: LGTM! Python version update is appropriate.The update to Python 3.11 aligns with the modernization effort and is consistent across all workflow files in this PR.
.github/workflows/publish_stable.yml (1)
29-29: LGTM! Consistent Python version update.The Python 3.11 update maintains consistency across all CI/CD workflows.
.github/workflows/license_tests.yml (1)
19-19: LGTM! Consistent Python version update.The Python 3.11 update is consistent with the modernization effort across all workflows.
requirements/requirements.txt (1)
1-1: The version constraint change is acceptable. The latest released version of ovos-plugin-manager is 2.1.0, so there are no breaking changes to verify in 2.2.0 or later versions. The plugin imports standard template classes (STT,AudioLanguageDetector) fromovos_plugin_manager.templateswith no version-specific code, making it compatible with future 2.x releases. Widening the constraint to<3.0.0is appropriate for allowing minor version updates.Likely an incorrect or invalid review comment.
.github/workflows/release_workflow.yml (2)
3-7: LGTM: Manual workflow trigger added.The
workflow_dispatchtrigger enables manual invocation of the workflow, which is useful for testing and on-demand releases.
46-49: Resolve Python version inconsistency between unit tests and release workflows.Unit tests explicitly run against both Python 3.10 and 3.11 (in
unit_tests.yml), but the release workflows use only 3.11. Clarify and align the minimum Python version across the project:
- Add
python_requirestosetup.pyto explicitly declare the minimum supported version- Either narrow
unit_tests.ymlto test only 3.11+ (if 3.10 support is not needed) or update the release workflows to support 3.10+Note:
setup.pycurrently has nopython_requiresfield and no Python version classifiers, leaving the package's compatibility unclear.setup.py (1)
73-73: LGTM: Improved package discoverability.Adding 'OpenVoiceOS' to the keywords improves package discoverability on PyPI and aligns with project branding.
| entry_points={'opm.stt': PLUGIN_ENTRY_POINT, | ||
| 'opm.stt.config': CONFIG_ENTRY_POINT, | ||
| 'opm.transformer.audio': LANG_PLUGIN_ENTRY_POINT} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ovos-plugin-manager version and entry point support
echo "=== Checking ovos-plugin-manager version requirement ==="
fd "requirements" --type f --exec rg -l "ovos-plugin-manager" {}
echo "---"
rg "ovos-plugin-manager" --type py --type txt
echo -e "\n=== Searching for documentation of opm.* entry points ==="
rg "opm\\.stt|opm\\.transformer" --type md --type rst --type txt
echo -e "\n=== Checking if old entry points are referenced elsewhere ==="
rg "mycroft\\.plugin\\.stt|neon\\.plugin\\.audio" --type py
echo -e "\n=== Examining setup.py for full context ==="
cat -n setup.py | head -100Repository: OpenVoiceOS/ovos-stt-plugin-fasterwhisper
Length of output: 3897
🌐 Web query:
ovos-plugin-manager entry points opm.stt opm.transformer.audio support
💡 Result:
Do you want (A) how to register plugins with ovos-plugin-manager (exact entry_point group names), (B) examples of STT / audio-transformer plugins that use those entry points, or (C) help adding support to a specific package? I can look up the exact entry-point group names and examples — confirm which of the above and I’ll fetch sources.
🌐 Web query:
ovos-plugin-manager entry point groups stt config transformer source code
💡 Result:
Here are the relevant source locations and what to look for:
-
ovos-plugin-manager (repo root / packaging — see setup.py / entry point namespace definitions). Use this to find the package's declared entry point groups. [1]
-
Example STT plugin repos (show how STT plugins declare entry points and config keys), e.g. ovos-stt-plugin-vosk and ovos-stt-plugin-fasterwhisper (inspect their setup.py for the entry_points section and their README for the expected mycroft.conf keys like "stt" -> "module" and plugin-specific sections). [2][3]
Quick pointers:
- In a plugin package, the entry point group for STT plugins is declared in setup.py (or pyproject) under entry_points; plugin config keys appear in their README (and are used under the "stt" section of mycroft/ovos config). See the repos above for exact source code and examples. [1][2][3]
References
- OpenVoiceOS / ovos-plugin-manager (GitHub). (github.com)
- OpenVoiceOS / ovos-stt-plugin-vosk (GitHub). (github.com)
- OpenVoiceOS / ovos-stt-plugin-fasterwhisper (GitHub). (github.com)
Would you like me to open the specific setup.py file(s) and copy the exact entry_points blocks for you?
Citations:
- 1: https://github.com/OpenVoiceOS/ovos-plugin-manager
- 2: https://github.com/OpenVoiceOS/ovos-stt-plugin-vosk?utm_source=openai
- 3: https://github.com/OpenVoiceOS/ovos-stt-plugin-fasterwhisper?utm_source=openai
🌐 Web query:
ovos-stt-plugin example entry_points opm.stt
💡 Result:
Example setup.py entry_points for registering an OVOS STT plugin with OPM:
setup.py snippet:
entry_points={
"opm.stt": [
"my_stt_plugin = my_package.my_module:MySTTPlugin"
]
},
- "opm.stt" is the OPM STT plugin entry-point group; the right-hand side is "name = package.module:ClassOrFactory". [1][2]
References:
[1] OpenVoiceOS/ovos-plugin-manager (OPM) repository. [2] Python packaging entry points documentation / examples.
🏁 Script executed:
# Check if there are any references to these entry point names in comments or docs
rg -i "entry.point|entry_point" --type md --type rst --type txt -A 2 -B 2
# Check for any migration guides or changelog
fd -e md -e rst -e txt | xargs grep -l -i "entry.point\|migration\|breaking" 2>/dev/null | head -5Repository: OpenVoiceOS/ovos-stt-plugin-fasterwhisper
Length of output: 67
🏁 Script executed:
# Search for how entry_points are defined in other OVOS plugins
rg -A 5 "entry_points\s*=" --type py | head -100
# Check if there's any documentation in this repo about the format
fd -e md -e txt | xargs cat 2>/dev/null | grep -i "entry.point" -A 3 -B 3Repository: OpenVoiceOS/ovos-stt-plugin-fasterwhisper
Length of output: 295
🌐 Web query:
Python setuptools entry_points list vs string format correct syntax
💡 Result:
Short answer: both are supported.
-
Preferred (setup.py dict + list): entry_points is a dict mapping groups to lists of "name = module:attr" strings.
Example:
entry_points={
"console_scripts": [
"mycmd = mypkg.cli:main",
],
"mypkg.plugins": [
"awesome = mypkg.plugins:awesome",
],
}
(This is the common/setup.py form.) [1][2] -
Alternate (INI-style string): entry_points may be a single string with .ini-style sections; setuptools/pkg_resources will parse sections into groups and lines like "name = module:attr". Example string:
"[console_scripts]\nmycmd = mypkg.cli:main\n[mypkg.plugins]\nawesome = mypkg.plugins:awesome\n"
(setuptools accepts this legacy string form and parses it as .ini sections). [2][5] -
Modern/pyproject.toml: prefer declaring entry points in pyproject.toml per PEP 621:
[project.scripts]
mycmd = "mypkg.cli:main"
[project.entry-points."mypkg.plugins"]
awesome = "mypkg.plugins:awesome"
(recommended for new projects). [3]
References:
- setuptools keywords / entry_points docs. [1]
- pkg_resources / entry point parsing (INI-string vs dict of lists). [2]
- PEP 621 / pyproject.toml entry-point format. [3]
🌐 Web query:
ovos-plugin-manager how to register plugins setup.py entry_points format list string
💡 Result:
Use setuptools entry points: the value is a dict mapping the plugin namespace to a list of strings "name = module:object". Example in setup.py (or setup.cfg):
setup.py example:
from setuptools import setup, find_packages
setup(
name="my_ovos_plugin",
version="0.1",
packages=find_packages(),
entry_points={
"ovos.plugin.namespace": [
"my_plugin = my_package.my_module:MyPluginClass",
],
},
)
Key points:
- entry_points is a dict; each value is a list of strings.
- Each string format: "plugin_name = package.module:CallableOrClass".
- setuptools writes these into egg-info/entry_points.txt and plugin loaders (e.g., stevedore/pkg_resources) discover them. (pypi.org)
If you want, tell me the exact namespace ovos-plugin-manager expects and I’ll adapt the snippet.
Citations:
🏁 Script executed:
# Look for other OVOS STT plugins to see how they define entry_points
fd "setup.py" --type f -x grep -A 5 "entry_points" {} +Repository: OpenVoiceOS/ovos-stt-plugin-fasterwhisper
Length of output: 355
Fix entry point format: values must be lists, not strings.
The entry point declarations have a critical format error. According to ovos-plugin-manager documentation, each entry point group must map to a list of strings, not individual strings:
Incorrect (current):
entry_points={'opm.stt': PLUGIN_ENTRY_POINT,
'opm.stt.config': CONFIG_ENTRY_POINT,
'opm.transformer.audio': LANG_PLUGIN_ENTRY_POINT}Correct:
entry_points={'opm.stt': [PLUGIN_ENTRY_POINT],
'opm.stt.config': [CONFIG_ENTRY_POINT],
'opm.transformer.audio': [LANG_PLUGIN_ENTRY_POINT]}The entry point group names (opm.stt, opm.stt.config, opm.transformer.audio) are correct for ovos-plugin-manager ≥2.1.0, but the values must be wrapped in lists for setuptools and plugin discovery to work properly.
🤖 Prompt for AI Agents
In @setup.py around lines 74 - 76, The entry_points mapping uses strings instead
of lists so setuptools/plugin discovery will fail; update the entry_points dict
so each group value is a list containing the respective constant (wrap
PLUGIN_ENTRY_POINT, CONFIG_ENTRY_POINT and LANG_PLUGIN_ENTRY_POINT each in a
list) while keeping the group keys ('opm.stt', 'opm.stt.config',
'opm.transformer.audio') unchanged.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #35 +/- ##
======================================
Coverage ? 74.72%
======================================
Files ? 2
Lines ? 91
Branches ? 0
======================================
Hits ? 68
Misses ? 23
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* Merge pull request #25 from OpenVoiceOS/renovate/configure chore: Configure Renovate * Increment Version to 0.3.2a1 * Update Changelog * chore(deps): update actions/setup-python action to v6 (#30) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Increment Version to 0.3.2a2 * Update Changelog * chore(deps): update dependency python to 3.14 (#26) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Increment Version to 0.3.2a3 * Update Changelog * chore(deps): update dependency ovos-plugin-manager to v2 (#33) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Increment Version to 0.3.2a4 * Update Changelog * feat: modernize (#35) * feat: modernize * feat: modernize * feat: modernize * feat: modernize * Increment Version to 0.4.0a1 * Update Changelog * Increment Version to 0.4.0a2 * chore(deps): update actions/checkout action to v6 (#27) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Increment Version to 0.4.0a3 * Update Changelog --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: JarbasAl <JarbasAl@users.noreply.github.com> Co-authored-by: JarbasAI <33701864+JarbasAl@users.noreply.github.com>
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.