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
Adds the pyeye1 MediaPipe eye-detection workflow, packages its model asset, integrates Python and browser execution, extends face detection with landmarks, and updates build, coverage, runner, npm, and maintenance configuration.
We reviewed changes in a919903...20e722f on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
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.
Pull Request Overview
The pull request is currently not up to standards due to high-complexity code and broad exception handling. While the functional requirements for eye detection and RetinaFace landmark decoding appear fulfilled across the Python and Rust modules, the et_ws_pyeye1.js file is flagged as a high-risk file: it contains a 'God Function' with excessive complexity and no test coverage, making it difficult to maintain and verify.
Furthermore, there is significant logic duplication between the new pyeye1 and the existing pyface1 JavaScript adapters. These issues, combined with the lack of a PR description, suggest that refactoring and documentation are required before merging.
About this PR
The PR includes significant changes to maintenance comments in Cargo.toml which are unrelated to the primary feature of eye detection. Consider moving these to a separate maintenance task.
The PR description is empty. Please provide context for the extensive changes across the eye detection and face detection modules to assist with future maintenance.
Test suggestions
Verify eye bounding box calculation from normalized landmarks in pyeye1
Verify RetinaFace landmark decoding and variance application in Rust face-detection
Verify RetinaFace landmark decoding and labelling in Python pyface1
Verify resolution of scoped npm package directories in the edge-toolkit library
Verify pyeye1 module availability in the web runner environment
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
Catching a broad Exception can hide unexpected bugs. Specify the exact exceptions you expect to handle from the JSON parsing or the infer() callback. Replace the broad except Exception as exc: on line 81 with specific exceptions such as json.JSONDecodeError or KeyError.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
This file shares a high amount of duplicate logic with pyface1/pkg/et_ws_pyface1.js. Common functions like init, cleanup, and log are identical. Propose moving these to a shared utility module to simplify maintenance and ensure consistent behavior across Pyodide adapters.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
The run method is handling too many responsibilities (connection, camera, model loading, and loop management). This function exceeds complexity thresholds and currently lacks test coverage. Refactor this into distinct lifecycle stages (e.g., initConnection, setupMediaDevices, loadFaceLandmarker) to improve readability and isolation.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 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.toml:
- Line 1406: Remove the duplicated TOML table headers, retaining only one
[tasks.fetch-eye1-rclone] declaration in .mise/config.toml and one
[asset."face_landmarker.task"] declaration in config/upstream-cache/data.toml;
preserve the settings under each table.
In `@services/ws-modules/face-detection/src/lib.rs`:
- Around line 556-568: Add a justified #[expect(clippy::single_call_fn, reason =
"...")] attribute to the private decode_retinaface_landmarks helper, explaining
that it remains separate for testability. Keep the helper’s implementation and
production caller unchanged.
In `@services/ws-web-runner/tests/modules.rs`:
- Line 153: Update the test setup around the pyeye1 case in the relevant test
function so a missing Python mise environment fails explicitly instead of
returning successfully. Ensure the pyeye1 case always executes with its required
environment and is not silently skipped or excluded from coverage.
🪄 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: b15ee848-d4c5-41be-a80d-85339cff4ae5
📥 Commits
Reviewing files that changed from the base of the PR and between a919903 and 20e722f.
Remove duplicated TOML table headers. Each repeated standard table declaration makes its file invalid TOML and blocks the associated mise or upstream-cache workflow.
.mise/config.toml#L1406-L1406: retain only one [tasks.fetch-eye1-rclone] header.
config/upstream-cache/data.toml#L30-L30: retain only one [asset."face_landmarker.task"] header.
📍 Affects 2 files
.mise/config.toml#L1406-L1406 (this comment)
config/upstream-cache/data.toml#L30-L30
🤖 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 1406, Remove the duplicated TOML table headers,
retaining only one [tasks.fetch-eye1-rclone] declaration in .mise/config.toml
and one [asset."face_landmarker.task"] declaration in
config/upstream-cache/data.toml; preserve the settings under each table.
decode_retinaface_landmarks has one production caller at Line 483. Keep it testable, but add a justified #[expect(clippy::single_call_fn, reason = "...")] instead of leaving the lint unacknowledged. As per coding guidelines, intentional single-call private functions must use a justified #[expect(...)].
🤖 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/face-detection/src/lib.rs` around lines 556 - 568, Add a
justified #[expect(clippy::single_call_fn, reason = "...")] attribute to the
private decode_retinaface_landmarks helper, explaining that it remains separate
for testability. Keep the helper’s implementation and production caller
unchanged.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not add pyeye1 to a silently skipped test.
When the Python mise environment is absent, this new case returns successfully at lines 155-160 without running or collecting coverage. Make the missing environment fail loudly, or ensure this test always runs with its required environment.
As per coding guidelines, "Do not skip, ignore, platform-disable, feature-disable, or early-return from tests without explicit user approval."
🤖 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 153, Update the test setup
around the pyeye1 case in the relevant test function so a missing Python mise
environment fails explicitly instead of returning successfully. Ensure the
pyeye1 case always executes with its required environment and is not silently
skipped or excluded from coverage.
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.
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Bug Fixes
Tests