Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 23 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)
📝 WalkthroughWalkthroughThis PR consolidates CI/CD workflows, migrates audio handling from Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GitHub as GitHub
participant PyPI as PyPI
participant Branch as dev Branch
Dev->>GitHub: Push to master
activate GitHub
GitHub->>GitHub: Trigger publish_stable workflow
GitHub->>GitHub: Execute publish_stable job<br/>(external TigreGotico workflow)
GitHub->>GitHub: publish_pypi job (after publish_stable)
GitHub->>GitHub: Checkout master, setup Python 3.11
GitHub->>GitHub: Build sdist and wheel
GitHub->>PyPI: Publish to PyPI with PYPI_TOKEN
PyPI-->>GitHub: Publish confirmed
GitHub->>GitHub: sync_dev job (after publish_stable)
GitHub->>Branch: Push master → dev
Branch-->>GitHub: Sync complete
deactivate GitHub
sequenceDiagram
participant Dev as Developer
participant GH as GitHub
participant Matrix as Matrix Channel
participant PyPI as PyPI
participant Release as Release Branch
Dev->>GH: Close/merge PR to dev or trigger workflow_dispatch
activate GH
GH->>GH: Trigger release_workflow
GH->>GH: publish_alpha job<br/>(delegates to publish-alpha.yml)
GH->>GH: notify job (if PR merged)<br/>Send to Matrix
Matrix-->>GH: Message sent
GH->>GH: publish_pypi job<br/>(after publish_alpha)
GH->>GH: Checkout dev, setup Python 3.11
GH->>GH: Build sdist and wheel
GH->>PyPI: Publish to PyPI
PyPI-->>GH: Publish confirmed
GH->>GH: propose_release job<br/>(after publish_alpha)
GH->>GH: Extract version, create release branch
GH->>Release: Push release-${VERSION} branch
GH->>GH: Open PR: release-${VERSION} → master
Release-->>GH: PR created
deactivate GH
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @JarbasAl. * #17 (comment) The following files were modified: * `ovos_audio_transformer_plugin_speechbrain_langdetect/__init__.py`
Docstrings generation was requested by @JarbasAl. * #17 (comment) The following files were modified: * `ovos_audio_transformer_plugin_speechbrain_langdetect/__init__.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ovos_audio_transformer_plugin_speechbrain_langdetect/__init__.py (1)
85-103: Return type inconsistency violates the plugin API contract + WAV/PCM bytes mismatch will corrupt audio.The
detect()method returns(audio_data, {})when only one language is invalid_langs(line 128), but the OVOSAudioLanguageDetectorcontract requires returning(lang_code, probability)as a(str, float)tuple. Callers expecting a language code will fail.Additionally, line 147 in
__main__passesaudio.get_wav_data()(WAV container bytes with RIFF header) todetect(), which wraps it intoAudioData(audio_data, 16000, 2)(line 122). TheAudioDataconstructor expects raw PCM frame bytes, not WAV bytes—the RIFF header will be misinterpreted as audio data, corrupting the signal.Fix the return type for the single-language case to match the API contract:
if len(valid) == 1: # no classification needed lang = valid[0] return lang, 1.0For the WAV/PCM issue: extract raw PCM frames from WAV data before passing to
detect(), or modify the wrapper to handle WAV format.
🤖 Fix all issues with AI agents
In @.github/workflows/publish_stable.yml:
- Around line 34-35: The workflow uses the deprecated ::set-output syntax in the
step that runs "echo \"::set-output name=version::$(python setup.py
--version)\"" with id "version"; either replace it with the environment file
method by writing the version to the GITHUB_OUTPUT file (e.g., echo
"version=$(python setup.py --version)" >> $GITHUB_OUTPUT) so later steps can
consume outputs, or remove the entire step (and its id "version") if that
computed version is not referenced by any subsequent step.
- Line 55: Replace the GitHub Action reference that currently uses the floating
ref "ad-m/github-push-action@master" with a specific, pinned release tag (e.g.,
the latest stable tag such as vX.Y.Z) so the workflow uses a fixed, versioned
commit; update the uses string in the workflow to that exact tag after verifying
the action's latest stable release on its repository.
In @.github/workflows/release_workflow.yml:
- Around line 3-20: The publish_alpha job currently runs on any pull_request
closed event; update the job-level condition for publish_alpha (job name:
publish_alpha) to only proceed when the PR was merged by adding an if that
checks github.event.pull_request.merged == true, and ensure downstream jobs that
depend on publish_alpha (e.g., publish_pypi, propose_release) either inherit
that merged check or also include an if: success() &&
github.event.pull_request.merged == true so the entire release pipeline cannot
run for unmerged PR closures.
- Around line 26-63: The "version" step uses the deprecated ::set-output syntax;
change it to write the output to the GITHUB_OUTPUT file (e.g., echo
"version=$(python setup.py --version)" >> $GITHUB_OUTPUT) and keep the step id
"version" so later steps can reference outputs as steps.version.outputs.version,
and replace the publish action reference uses:
pypa/gh-action-pypi-publish@master with a pinned release track (e.g.,
pypa/gh-action-pypi-publish@release/v1 or another stable tag) in the "Publish to
PyPI" step to avoid using the master branch.
In @.github/workflows/unit_tests.yml:
- Around line 4-10: Update the GitHub Actions workflow's pull_request branch
filter so PRs to the repo default branch trigger the job: in the workflow YAML
where the pull_request trigger is defined (the pull_request: branches: list),
add "dev" alongside "master" (or replace with the correct default branch name)
so the branches array includes both "master" and "dev" under the pull_request
key.
- Around line 11-14: The reusable workflow reference under the py_build_tests
job is pinned to a non-deterministic ref and the python_version is too low;
update the uses value (the uses field on the py_build_tests job) to a specific
tag or commit SHA instead of @master and change python_version from "3.8" to
"3.9" (or the minimum Python required by pinned dependencies) so the job is
deterministic and compatible with huggingface_hub==0.36.0.
In @ovos_audio_transformer_plugin_speechbrain_langdetect/__init__.py:
- Around line 3-8: The shim that sets torchaudio.list_audio_backends currently
returns an array with an empty string, which is not a valid backend name; change
the fallback assigned to torchaudio.list_audio_backends (the lambda defined in
__init__.py) so it returns an empty list [] to correctly indicate no available
backends instead of [''].
In @requirements.txt:
- Around line 1-8: The requirements file currently hard-pins
huggingface_hub==0.36.0 and leaves torchaudio and speechbrain unconstrained
which risks future breakage; verify whether the referenced SpeechBrain issue
exists and if the exact pin is required, otherwise relax huggingface_hub to a
range (for example huggingface_hub>=0.36.0,<1.0.0) and add conservative upper
bounds for torchaudio and speechbrain (e.g., torchaudio>=2.1.0,<3.0.0 and
speechbrain>=1.0.3,<2.0.0) in requirements.txt, then run the project’s test
suite and any integration flows that use SpeechBrain/huggingface_hub to confirm
compatibility before committing the change.
🧹 Nitpick comments (7)
.github/workflows/conventional-label.yaml (1)
8-10: Pin the action to a commit SHA (avoid floating@v1)
Using a floating ref increases supply-chain risk and can cause non-reproducible CI behavior.Proposed change
steps: - - uses: bcoe/conventional-release-labels@v1 + - uses: bcoe/conventional-release-labels@<PINNED_COMMIT_SHA>.github/workflows/build_tests.yml (1)
10-16: Consider updating action majors + quotepython-versionto avoid YAML numeric parsing quirks.Even though only the Python version changed, this job still uses very old
actions/checkout@v2andactions/setup-python@v1. Also,3.11is a YAML number; quoting avoids edge cases.Proposed update
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 with: ref: ${{ github.head_ref }} - name: Setup Python - uses: actions/setup-python@v1 + uses: actions/setup-python@v5 with: - python-version: 3.11 + python-version: "3.11"ovos_audio_transformer_plugin_speechbrain_langdetect/__init__.py (1)
108-113:__main__example likely passes the wrong byte format intodetect().Given the new wrapping behavior in
detect(), passingaudio.get_wav_data()(Line 112) may be incorrect ifAudioData(...)expects raw PCM. Consider passing theAudioDatainstance directly (or raw frames), e.g.s.detect(audio, ...)if the public API allows it..github/workflows/release_workflow.yml (1)
65-108:propose_releaselikely needs explicit permissions and basic idempotency (avoid duplicate PRs/branch push failures).Consider adding:
permissions: contents: write, pull-requests: write(at workflow or job level).- A guard so re-runs don’t fail when branch or PR already exists (GitHub API returns 422).
.github/workflows/publish_stable.yml (3)
9-9: Pin the reusable workflow to a specific version or commit SHA.Using
@masterfor the external workflow means this job depends on potentially changing upstream code, which can introduce breaking changes or security risks without notice.🔒 Recommended fix
- uses: TigreGotico/gh-automations/.github/workflows/publish-stable.yml@master + uses: TigreGotico/gh-automations/.github/workflows/publish-stable.yml@v1.0.0 # or specific commit SHA
38-38: Consider usingpython -m buildfor distribution packages.While
python setup.py sdist bdist_wheelworks, the modern and recommended approach is to usepython -m build, which you've already installed in line 32.♻️ Proposed refactor
- name: Build Distribution Packages run: | - python setup.py sdist bdist_wheel + python -m build
40-40: Pin the PyPI publish action to a specific version.Using
@mastercreates a dependency on potentially changing upstream code. Pin to a stable release tag for better security and stability.🔒 Recommended fix
- uses: pypa/gh-action-pypi-publish@master + uses: pypa/gh-action-pypi-publish@release/v1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/build_tests.yml.github/workflows/conventional-label.yaml.github/workflows/dev2master.yml.github/workflows/license_tests.yml.github/workflows/publish_alpha.yml.github/workflows/publish_build.yml.github/workflows/publish_major.yml.github/workflows/publish_minor.yml.github/workflows/publish_stable.yml.github/workflows/release_workflow.yml.github/workflows/unit_tests.ymlovos_audio_transformer_plugin_speechbrain_langdetect/__init__.pyrequirements.txtscripts/bump_alpha.pyscripts/bump_build.pyscripts/bump_major.pyscripts/bump_minor.pyscripts/remove_alpha.pysetup.py
💤 Files with no reviewable changes (10)
- scripts/bump_minor.py
- .github/workflows/publish_alpha.yml
- scripts/bump_alpha.py
- .github/workflows/publish_minor.yml
- .github/workflows/publish_major.yml
- .github/workflows/publish_build.yml
- scripts/remove_alpha.py
- scripts/bump_build.py
- scripts/bump_major.py
- .github/workflows/dev2master.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/release_workflow.yml
54-54: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
.github/workflows/publish_stable.yml
34-34: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🪛 GitHub Actions: Run Unit Tests
setup.py
[error] 1-1: python build_test/setup.py bdist_wheel sdist failed. Are there relative paths in setup.py?
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: license_tests / license_tests
- GitHub Check: build_tests
🔇 Additional comments (5)
.github/workflows/license_tests.yml (2)
10-12: Parameter is supported; pinning unavailable due to upstream repository having no releases.The
packages-excludeparameter is correctly supported by the external workflow and your custom regex pattern will properly override the upstream default. However, using@masterfor the workflow reference remains a stability concern. Theneongeckocom/.githubrepository currently has no release tags, making it impossible to pin to a specific version. Consider opening an issue with the upstream repository to request creating version releases for better reproducibility and stability.
1-8: LGTM: workflow_dispatch trigger added.The addition of
workflow_dispatchenables manual triggering of the license tests, which is a useful feature for on-demand testing. The pull_request branch targetingmasteris consistent with the repository's test automation pattern—unit_tests.ymlalso targetsmasterfor pull_request events, while dev branch is reserved for release workflows inrelease_workflow.yml..github/workflows/conventional-label.yaml (1)
2-4: Add explicit permissions block, but useissues: writeinstead ofpull-requests: write
pull_request_targetruns in the base repo context; hardening with least-privilege permissions is good practice. However, thebcoe/conventional-release-labelsaction requiresissues: writeto create/update/remove labels—notpull-requests: write. Using the wrong permission scope will cause "Resource not accessible by integration" errors.Proposed change
on: pull_request_target: types: [ opened, edited ] name: conventional-release-labels +permissions: + contents: read + issues: write jobs: label: runs-on: ubuntu-latestLikely an incorrect or invalid review comment.
setup.py (1)
52-73: Entry-point group is correct; no action needed.The
opm.transformer.audiogroup is the documented entry-point group for OVOS audio transformers (confirmed by ovos-plugin-manager). There is noneon.plugin.audiogroup in the OVOS ecosystem, and the plugin correctly inherits fromAudioLanguageDetectorinovos_plugin_manager.templates.transformers, confirming proper integration with the OVOS plugin system.Likely an incorrect or invalid review comment.
.github/workflows/publish_stable.yml (1)
13-13: Version file exists and path is correct. The fileovos_audio_transformer_plugin_speechbrain_langdetect/version.pyexists at the path specified in the workflow. While the PR removes local versioning scripts, the version.py file itself is still present and is the correct reference for the publishing workflow.
1 participant
Summary by CodeRabbit
New Features
Improvements
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.