Skip to content

Add speech detection#94

Merged
pierre-tenedero merged 1 commit into
mainfrom
speech-detection
Jul 21, 2026
Merged

Add speech detection#94
pierre-tenedero merged 1 commit into
mainfrom
speech-detection

Conversation

@pierre-tenedero

@pierre-tenedero pierre-tenedero commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added real-time browser-based Speech1 microphone speech detection with a live waveform, confidence scoring, and websocket inference event output.
    • Added Speech1 ONNX model support with checksum-verified download and integration into the model prefetch flow.
    • Introduced a new Speech1 workspace module with Python wheel build support and runtime enablement.
  • Bug Fixes
    • Improved coverage aggregation and Python module resolution so Speech1 test runs are reflected in combined reports.
  • Tests
    • Added unit tests for silence vs speech detection, short-noise rejection, input validation, and websocket event payload formatting.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

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/*
Adds Pyodide setup, microphone capture, recurrent ONNX inference, websocket reporting, UI controls, waveform rendering, and cleanup.
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.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BrowserUI
  participant et_ws_pyspeech1
  participant Pyodide
  participant speech_detection
  participant ONNXRuntime
  participant WebSocket
  BrowserUI->>et_ws_pyspeech1: start recording
  et_ws_pyspeech1->>Pyodide: run capture and detection
  et_ws_pyspeech1->>ONNXRuntime: infer audio chunks
  ONNXRuntime-->>Pyodide: return probabilities
  Pyodide->>speech_detection: summarize probabilities
  speech_detection->>WebSocket: publish client event JSON
  speech_detection-->>BrowserUI: render detection status
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding speech detection functionality.
Docstring Coverage ✅ Passed Docstring coverage is 87.18% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch speech-detection

Comment @coderabbitai help to get the list of available commands.

@codacy-production

codacy-production Bot commented Jul 21, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 6 high · 2 medium · 3 minor

Alerts:
⚠ 11 issues (≤ 0 issues of at least minor severity)

Results:
11 new issues

Category Results
Documentation 1 minor
ErrorProne 6 high
CodeStyle 1 minor
Complexity 2 medium
1 minor

View in Codacy

🟢 Metrics 136 complexity · 5 duplication

Metric Results
Complexity 136
Duplication 5

View in Codacy

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
.mise/config.toml (1)

416-418: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff

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.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • .mise/config.python.toml
  • .mise/config.toml
  • config/coverage.toml
  • config/pyrefly.toml
  • config/upstream-cache/data.toml
  • data/model-modules/model-speech1/pkg/package.json
  • pyproject.toml
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-modules/pyspeech1/pyspeech1/__init__.py
  • services/ws-modules/pyspeech1/pyspeech1/speech_detection.py
  • services/ws-modules/pyspeech1/tests/test_speech_detection.py
  • services/ws-web-runner/tests/modules.rs

Comment thread .mise/config.python.toml
description = "Build the pyspeech1 Python Silero VAD workflow module"
dir = "services/ws-modules/pyspeech1"
run = """
rm -f pkg/*.whl

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use the mise-managed coreutils for wheel cleanup.

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.

Source: Path instructions

Comment thread .mise/config.toml Outdated

[tasks.fetch-speech1-rclone]
run = """
rclone copyto {{ vars.speech1_src }} {{ vars.speech1_dst }} --http-url https://huggingface.co --progress --ignore-existing

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

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.

Proposed change
-rclone copyto {{ vars.speech1_src }} {{ vars.speech1_dst }} --http-url https://huggingface.co --progress --ignore-existing
+rclone copyto {{ vars.speech1_src }} {{ vars.speech1_dst }} {{ vars.hf_http }}

Add alongside gh_http:

 gh_http = "--http-url https://github.com --progress --ignore-existing"
+hf_http = "--http-url https://huggingface.co --progress --ignore-existing"
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.

Suggested change
rclone copyto {{ vars.speech1_src }} {{ vars.speech1_dst }} --http-url https://huggingface.co --progress --ignore-existing
rclone copyto {{ vars.speech1_src }} {{ vars.speech1_dst }} {{ vars.hf_http }}
🤖 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 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.

Source: Coding guidelines

Comment thread config/coverage.toml
button.style.cursor = "wait";
button.style.opacity = "0.72";
button.dataset.state = "recording";
button.querySelector("span:last-child").textContent = "Recording…";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Non-ASCII ellipsis in UI string.

"Recording…" uses the U+2026 ellipsis character. Replace with ASCII ....

Proposed change
-  button.querySelector("span:last-child").textContent = "Recording…";
+  button.querySelector("span:last-child").textContent = "Recording...";
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.

Suggested change
button.querySelector("span:last-child").textContent = "Recording";
button.querySelector("span:last-child").textContent = "Recording...";
🧰 Tools
🪛 ast-grep (0.44.1)

[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.

Source: Coding guidelines

Comment thread services/ws-modules/pyspeech1/pyspeech1/speech_detection.py Outdated
#[case::video1("et-ws-video1", Language::Rust)]
#[case::pyface1("et-ws-pyface1", Language::Python)]
#[case::pyeye1("et-ws-pyeye1", Language::Python)]
#[case::pyspeech1("et-ws-pyspeech1", Language::Python)]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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.

Source: Path instructions

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.88372% with 13 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ws-modules/pyspeech1/pyspeech1/speech_detection.py 84.52% 13 Missing ⚠️

📢 Thoughts on this report? Let us know!

@pierre-tenedero
pierre-tenedero force-pushed the speech-detection branch 2 times, most recently from 401a9d1 to ac8caae Compare July 21, 2026 08:08
@deepsource-io

deepsource-io Bot commented Jul 21, 2026

Copy link
Copy Markdown

DeepSource Code Review

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.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Coverage  

Code Review Summary

Analyzer Status Updated (UTC) Details
C# Jul 21, 2026 8:58a.m. Review ↗
C & C++ Jul 21, 2026 8:58a.m. Review ↗
Docker Jul 21, 2026 8:58a.m. Review ↗
Java Jul 21, 2026 8:58a.m. Review ↗
JavaScript Jul 21, 2026 8:58a.m. Review ↗
Python Jul 21, 2026 8:58a.m. Review ↗
Rust Jul 21, 2026 8:58a.m. Review ↗
Secrets Jul 21, 2026 8:58a.m. Review ↗
Code coverage Jul 21, 2026 9:26a.m. Review ↗

Code Coverage Summary

Language Line Coverage (New Code) Line Coverage (Overall)
Aggregate
84.9%
57.1%
[▲ up 0.1% from main]
Python
84.9%
69.7%
[▲ up 3.5% from main]
Rust -
56.5%
[▼ down 0.1% from main]

➟ Additional coverage metrics may have been reported. See full coverage report ↗


Important

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.

let cfg;
let runtime = null;

export default async function init() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'


Found non-compliant syntax. Confirm that there are no syntax errors before committing your code to a version control system.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (16)
  • .mise/config.coverage.toml
  • .mise/config.python.toml
  • .mise/config.toml
  • config/coverage.toml
  • config/pyrefly.toml
  • config/upstream-cache/data.toml
  • data/model-modules/model-speech1/pkg/package.json
  • pyproject.toml
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-modules/pyspeech1/pyspeech1/__init__.py
  • services/ws-modules/pyspeech1/pyspeech1/speech_detection.py
  • services/ws-modules/pyspeech1/tests/test_speech_detection.py
  • services/ws-web-runner/tests/modules.rs
🚧 Files skipped from review as they are similar to previous changes (14)
  • pyproject.toml
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-modules/pyspeech1/pyspeech1/init.py
  • data/model-modules/model-speech1/pkg/package.json
  • config/pyrefly.toml
  • config/coverage.toml
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-web-runner/tests/modules.rs
  • services/ws-modules/pyspeech1/tests/test_speech_detection.py
  • config/upstream-cache/data.toml
  • .mise/config.python.toml
  • .mise/config.toml
  • services/ws-modules/pyspeech1/pyspeech1/speech_detection.py

Comment on lines +145 to +149
# The pyface1 + pyeye1 + pyspeech1 pytest runs and the web-runner integration test (which drops the modules'
# .coverage files into target/pycov/ when ET_TEST_COVERAGE is set) all feed `coverage combine`, which merges
# them via config/coverage.toml's `[paths]` remap into a single Cobertura file. pytest-cov is a plugin, so it
# rides in each module's uv `dev` group -- the same venv as pytest -- and `uv run` (a mise-managed tool) installs
# both into the one project venv. UV_PYTHON mirrors test-pyface1's mise-CPython pin.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the coverage configuration section name.

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].

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
services/ws-modules/pyspeech1/tests/test_speech_detection.py (1)

41-49: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Cover the complete event payload contract.

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.

Suggested assertions
         event = json.loads(client_event_json(payload))
+        self.assertEqual(event["type"], "et-client-event")
         self.assertEqual(event["capability"], "speech_detection")
         self.assertEqual(event["action"], "inference")
         self.assertEqual(event["details"]["label"], "speech")
+        self.assertEqual(event["details"]["source_sample_rate"], 48_000)
         self.assertEqual(event["details"]["model_sample_rate"], SAMPLE_RATE)
+        self.assertEqual(event["details"]["recorded_seconds"], 5.0)
+        self.assertIn("threshold", event["details"])
🤖 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/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.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (16)
  • .mise/config.coverage.toml
  • .mise/config.python.toml
  • .mise/config.toml
  • config/coverage.toml
  • config/pyrefly.toml
  • config/upstream-cache/data.toml
  • data/model-modules/model-speech1/pkg/package.json
  • pyproject.toml
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-modules/pyspeech1/pyspeech1/__init__.py
  • services/ws-modules/pyspeech1/pyspeech1/speech_detection.py
  • services/ws-modules/pyspeech1/tests/test_speech_detection.py
  • services/ws-web-runner/tests/modules.rs
🚧 Files skipped from review as they are similar to previous changes (13)
  • pyproject.toml
  • data/model-modules/model-speech1/pkg/package.json
  • config/pyrefly.toml
  • config/coverage.toml
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-modules/pyspeech1/pyspeech1/init.py
  • config/upstream-cache/data.toml
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-web-runner/tests/modules.rs
  • .mise/config.coverage.toml
  • .mise/config.toml
  • .mise/config.python.toml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (16)
  • .mise/config.coverage.toml
  • .mise/config.python.toml
  • .mise/config.toml
  • config/coverage.toml
  • config/pyrefly.toml
  • config/upstream-cache/data.toml
  • data/model-modules/model-speech1/pkg/package.json
  • pyproject.toml
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • services/ws-modules/pyspeech1/pkg/et_ws_pyspeech1.js
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-modules/pyspeech1/pyspeech1/__init__.py
  • services/ws-modules/pyspeech1/pyspeech1/speech_detection.py
  • services/ws-modules/pyspeech1/tests/test_speech_detection.py
  • services/ws-web-runner/tests/modules.rs
🚧 Files skipped from review as they are similar to previous changes (15)
  • pyproject.toml
  • config/pyrefly.toml
  • data/model-modules/model-speech1/pkg/package.json
  • services/ws-modules/pyspeech1/pyproject.toml
  • services/ws-modules/pyspeech1/pkg/package.json
  • services/ws-modules/pyspeech1/pkg/.gitignore
  • config/coverage.toml
  • services/ws-modules/pyspeech1/tests/test_speech_detection.py
  • .mise/config.coverage.toml
  • services/ws-web-runner/tests/modules.rs
  • config/upstream-cache/data.toml
  • services/ws-modules/pyspeech1/pyspeech1/init.py
  • services/ws-modules/pyspeech1/pyspeech1/speech_detection.py
  • .mise/config.python.toml
  • .mise/config.toml

await installLocalWheel(`${pkg.name.replace(/-/g, "_")}-${pkg.version}-py3-none-any.whl`);
const { installWheel: installEtWs } = await import("/modules/et-ws/et_ws.js");
await installEtWs(pyodide);
if (globalThis.__etPyCov) await globalThis.__etPyCov.start(pyodide, "pyspeech1");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/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/bash
set -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/bash
set -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/bash
set -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.

@pierre-tenedero
pierre-tenedero merged commit 15131c3 into main Jul 21, 2026
41 of 46 checks passed
@pierre-tenedero
pierre-tenedero deleted the speech-detection branch July 21, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants