You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Use the following commands to manage reviews:
@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.
Use the checkboxes below for quick actions:
▶️ Resume reviews
🔍 Trigger review
📝 Walkthrough
Walkthrough
The change adds a Speech1 ONNX speech-detection module with Python processing, browser microphone capture and inference, model asset packaging, websocket reporting, and workspace test/build integration.
Changes
Speech1 module
Layer / File(s)
Summary
Speech1 model asset wiring .mise/config.toml, config/upstream-cache/data.toml, data/model-modules/model-speech1/pkg/package.json
Adds Speech1 model metadata, checksum validation, download variables, and fetch-task integration.
Python detection contract and orchestration services/ws-modules/pyspeech1/pyproject.toml, services/ws-modules/pyspeech1/pyspeech1/*, services/ws-modules/pyspeech1/tests/*
Adds probability summarization, event payload creation, status helpers, orchestration, and unit tests.
Browser capture and ONNX workflow services/ws-modules/pyspeech1/pkg/*
Workspace, validation, and test integration pyproject.toml, .mise/config.python.toml, .mise/config.coverage.toml, config/{coverage,pyrefly}.toml, services/ws-web-runner/tests/modules.rs
Registers the module for workspace builds, dependency synchronization, testing, type checking, coverage, and browser-runner validation.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer TIP This summary will be updated as you push new changes.
Consider mirroring speech1 through the hf-cache release instead of fetching direct from HuggingFace.
speech1_src downloads directly from huggingface.co, but the face1_asset comment in this same file documents that HuggingFace 429s the CI runner IP pool during prefetch, which is why face1/eye1 are mirrored to the hf-cache-v<N> release. Fetching speech1 directly reintroduces that flaky-fetch risk in every lane that hits the model concurrently.
As per coding guidelines: "For flaky network fetches, mirror assets through the upstream-cache mechanism instead of adding generic retry wrappers".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.mise/config.toml around lines 416 - 418, Update the speech1 asset
configuration by mirroring it through the existing hf-cache release mechanism
instead of fetching directly from HuggingFace. Change speech1_src while
preserving speech1_asset and speech1_dst, and follow the established face1/eye1
configuration pattern and cache versioning symbols in .mise/config.toml.
Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.mise/config.python.toml:
- Line 156: Update the wheel cleanup command in the mise task to invoke the
repository’s mise-managed coreutils `rm` command or established helper instead
of unqualified `rm`, while preserving removal of matching files under pkg.
In @.mise/config.toml:
- Line 1441: In the fetch-face1-rclone task, replace the inline HuggingFace
flags in the rclone copyto command with a new vars.hf_http variable. Define
hf_http alongside the existing gh_http variable with the exact --http-url,
--progress, and --ignore-existing options, preserving the command’s behavior and
keeping the line within 120 characters.
In `@config/coverage.toml`:
- Line 16: Add a pyspeech1 pytest execution and coverage-copy step to the
combined coverage task in the coverage configuration, alongside the existing
pyface1 and pyeye1 steps, so its results are included in coverage-python.xml.
Preserve the existing pyspeech1 path alias and task behavior.
In `@services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js`:
- Line 148: Update the recording-state UI text assigned to the button span in
the recording control flow, replacing the Unicode ellipsis in “Recording…” with
three ASCII periods while preserving the existing wording and behavior.
In `@services/ws-modules/pyspeech1/pyspeech1/speech_detection.py`:
- Around line 3-7: Replace the Python 3.11-only datetime.UTC usage in
speech_detection.py with datetime.timezone.utc, updating the import and all
references while preserving requires-python >=3.10. The pyproject.toml
requires-python declaration needs no direct change.
In `@services/ws-web-runner/tests/modules.rs`:
- Line 175: Update the hardware_module_load_fails test flow for the pyspeech1
case so a missing Python mise environment fails loudly instead of returning
success. Remove or change the existing environment guard to report an error,
while preserving normal test behavior when the environment is available.
---
Nitpick comments:
In @.mise/config.toml:
- Around line 416-418: Update the speech1 asset configuration by mirroring it
through the existing hf-cache release mechanism instead of fetching directly
from HuggingFace. Change speech1_src while preserving speech1_asset and
speech1_dst, and follow the established face1/eye1 configuration pattern and
cache versioning symbols in .mise/config.toml.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 79a5f639-c61e-4856-85ca-9db962157cee
📥 Commits
Reviewing files that changed from the base of the PR and between a83a08d and e0657ff.
Line 156 invokes an unqualified rm in a new mise task. Use the repository's managed coreutils command or helper so tool resolution remains consistent across supported environments.
As per path instructions, mise task bodies must use mise-managed tools rather than assuming host utilities; use coreutils, rg, find/xargs, and goawk as prescribed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.mise/config.python.toml at line 156, Update the wheel cleanup command in
the mise task to invoke the repository’s mise-managed coreutils `rm` command or
established helper instead of unqualified `rm`, while preserving removal of
matching files under pkg.
Line exceeds 120 chars; extract the HuggingFace flags into a var.
This line is 122 characters. The fetch-face1-rclone task keeps its command on one line by using {{ vars.gh_http }}; add an hf_http var (--http-url https://huggingface.co --progress --ignore-existing) and reuse it here to stay under the limit and match the existing pattern.
As per coding guidelines: "Keep lines at or below 120 characters, including comments, documentation, configuration strings, tables".
📝 Committable suggestion
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.mise/config.toml at line 1441, In the fetch-face1-rclone task, replace the
inline HuggingFace flags in the rclone copyto command with a new vars.hf_http
variable. Define hf_http alongside the existing gh_http variable with the exact
--http-url, --progress, and --ignore-existing options, preserving the command’s
behavior and keeping the line within 120 characters.
As per coding guidelines: "Keep hand-written repository content ASCII-only; use ASCII alternatives for punctuation and symbols."
📝 Committable suggestion
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[error] 148-148: React's useState should not be directly called
Context: setStatus(py.starting_status())
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js` at line 148, Update the
recording-state UI text assigned to the button span in the recording control
flow, replacing the Unicode ellipsis in “Recording…” with three ASCII periods
while preserving the existing wording and behavior.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not add a silently skippable test case.
This case enters hardware_module_load_fails, whose existing environment guard returns successfully when the Python mise environment is absent. The new test can therefore disappear from CI without failing; make a missing environment an error or ensure the test task always loads it.
As per path instructions, tests must not be skipped or conditionally disabled without explicit approval; missing tools must fail loudly rather than self-skip.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-web-runner/tests/modules.rs` at line 175, Update the
hardware_module_load_fails test flow for the pyspeech1 case so a missing Python
mise environment fails loudly instead of returning success. Remove or change the
existing environment guard to report an error, while preserving normal test
behavior when the environment is available.
We reviewed changes in 80b5b22...a5282fe on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.mise/config.coverage.toml:
- Around line 145-149: Update the coverage configuration reference in the
comment near the pytest coverage description to use the actual
[tool.coverage.paths] section name instead of [paths].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cdbfcdc6-8dac-4803-8645-722a9befd253
📥 Commits
Reviewing files that changed from the base of the PR and between 401a9d1 and ac8caae.
The comment refers to config/coverage.toml's [paths] section, but the actual section is [tool.coverage.paths]. Update the reference to avoid sending maintainers to a nonexistent section.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.mise/config.coverage.toml around lines 145 - 149, Update the coverage
configuration reference in the comment near the pytest coverage description to
use the actual [tool.coverage.paths] section name instead of [paths].
This test verifies the envelope, label, and model sample rate, but it would still pass if source_sample_rate, recorded_seconds, or threshold were omitted or formatted incorrectly. Assert those downstream fields as well, plus the event type.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-modules/pyspeech1/tests/test_speech_detection.py` around lines 41
- 49, Extend test_event_uses_speech_detection_capability to assert the complete
event payload contract: event type, source_sample_rate, recorded_seconds, and
threshold, using the expected values from the constructed payload. Preserve the
existing envelope, capability, action, label, and model_sample_rate assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@services/ws-modules/pyspeech1/tests/test_speech_detection.py`:
- Around line 41-49: Extend test_event_uses_speech_detection_capability to
assert the complete event payload contract: event type, source_sample_rate,
recorded_seconds, and threshold, using the expected values from the constructed
payload. Preserve the existing envelope, capability, action, label, and
model_sample_rate assertions.
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e13c368-c524-44ad-adfc-5754b9758301
📥 Commits
Reviewing files that changed from the base of the PR and between ac8caae and 2949ce0.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js`:
- Line 35: Move the globalThis.__etPyCov.stop() call in run() into a finally
block so coverage is stopped and saved even when setup fails. Ensure the finally
executes after recordAndDetect() completes, preserving coverage for the
interactive speech flow while retaining the existing start behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 764fad51-119c-4632-8059-926522a5a0ca
📥 Commits
Reviewing files that changed from the base of the PR and between 2949ce0 and a5282fe.
#!/bin/bash# Inspect the coverage helper API and how start/stop are expected to be paired.
fd -t f -e js -e mjs | xargs rg -nP '__etPyCov|\.start\s*\(|\.stop\s*\(' -g '!**/node_modules/**' -C2
# Check what the browser-runner test exercises (init only vs init+run).
fd modules.rs services/ws-web-runner/tests --exec cat -n {}
Repository: edge-toolkit/core
Length of output: 26541
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Read the pyspeech1 module around init/start/stop and the event-driven paths.
sed -n '1,220p' services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js
printf'\n--- pycov helper ---\n'
sed -n '1,120p' services/ws-web-runner/src/shims/pycov.js
printf'\n--- other pyodide modules for pattern comparison ---\n'
sed -n '1,180p' services/ws-modules/pyface1/pkg/et_ws_pyface1.js
printf'\n---\n'
sed -n '1,180p' services/ws-modules/pyeye1/pkg/et_ws_pyeye1.js
printf'\n---\n'
sed -n '1,180p' services/ws-modules/pydata1/pkg/et_ws_pydata1.js
Repository: edge-toolkit/core
Length of output: 29918
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Inspect the browser-runner test coverage collection flow to see whether module coverage# is expected to stop before user-driven callbacks and after module setup completes.
sed -n '1,220p' services/ws-web-runner/tests/modules.rs
Repository: edge-toolkit/core
Length of output: 11210
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Inspect the pyspeech1 module and compare with similar Pyodide modules.
sed -n '1,220p' services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js
printf'\n--- pycov helper ---\n'
sed -n '1,120p' services/ws-web-runner/src/shims/pycov.js
printf'\n--- pyface1 ---\n'
sed -n '1,180p' services/ws-modules/pyface1/pkg/et_ws_pyface1.js
printf'\n--- pyeye1 ---\n'
sed -n '1,180p' services/ws-modules/pyeye1/pkg/et_ws_pyeye1.js
printf'\n--- pydata1 ---\n'
sed -n '1,180p' services/ws-modules/pydata1/pkg/et_ws_pydata1.js
printf'\n--- tests/modules.rs ---\n'
sed -n '1,220p' services/ws-web-runner/tests/modules.rs
Repository: edge-toolkit/core
Length of output: 40985
Move __etPyCov.stop() into finally. A setup error in run() skips the .coverage save/upload; if you need the interactive speech flow covered too, the current stop point ends tracing before recordAndDetect() runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js` at line 35, Move the
globalThis.__etPyCov.stop() call in run() into a finally block so coverage is
stopped and saved even when setup fails. Ensure the finally executes after
recordAndDetect() completes, preserving coverage for the interactive speech flow
while retaining the existing start behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit