Improve coverage by jayvdb · Pull Request #76 · edge-toolkit/core

jayvdb · 2026-07-09T02:15:01Z

No description provided.

deepsource-io · 2026-07-09T02:15:25Z

DeepSource Code Review

We reviewed changes in 4ac20d6...a85e4b6 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 ↗

Important

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.

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Updated (UTC)	Details
C#	Jul 10, 2026 8:59a.m.	Review ↗
C & C++	Jul 10, 2026 8:59a.m.	Review ↗
Docker	Jul 10, 2026 8:59a.m.	Review ↗
Java	Jul 10, 2026 8:59a.m.	Review ↗
JavaScript	Jul 10, 2026 8:59a.m.	Review ↗
Python	Jul 10, 2026 8:59a.m.	Review ↗
Rust	Jul 10, 2026 8:59a.m.	Review ↗
Secrets	Jul 10, 2026 8:59a.m.	Review ↗

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.

codacy-production · 2026-07-09T02:17:49Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics -10 complexity · 4 duplication

Metric Results

Complexity -10

Duplication 4

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

codacy-production

Pull Request Overview

The PR successfully unifies the Rust testing workflow under cargo-nextest and mise, providing better serialization for resource-heavy tests. However, there is a critical issue in the .mise/config.toml where a Windows-specific variable is used globally without a default value, which will cause task failures on Linux and macOS.

Additionally, while the change to ls-lint.yaml fixes a race condition, it introduces a maintenance burden by requiring literal paths for build directories. This refactoring is incorrectly titled 'Improve coverage', as it focuses on infrastructure rather than testing logic.

About this PR

No PR description was provided to explain the motivation behind the migration to cargo-nextest or the specific fix for ls-lint race conditions.
The PR title 'Improve coverage' is imprecise. The changes refactor test runners and infrastructure rather than adding test cases to increase code coverage metrics.

Test suggestions

Verify cargo-test task executes cargo-nextest with the workspace flag and correct config path
Verify web-runner test-group is configured to max-threads = 1 in nextest.toml
Verify cargo-nextest is registered as a GitHub-backend tool in mise config
Verify et-ws-web-runner is excluded on Windows gnullvm target via cargo_ws_excludes variable

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

codacy-production · 2026-07-09T02:17:51Z

+# except on the gnullvm Windows target, which cannot build et-ws-web-runner (rusty_v8 ships no gnullvm prebuilt);
+# there it expands to `--exclude et-ws-web-runner`, so the crate is dropped from the build (the config.mingw/msvc
+# envs clear the var, so those lanes build + run it against the msvc rusty_v8 prebuilt).
+run = "cargo nextest run --workspace {{ vars.cargo_ws_excludes }} --config-file config/nextest.toml"


_{🔴 HIGH RISK}

The cargo-test task references {{ vars.cargo_ws_excludes }}, but this variable is not defined in the base config.toml. This will cause mise to fail with a template error on non-Windows platforms. Define a default empty value (e.g., cargo_ws_excludes = "") in the global [vars] section of config.toml.

codacy-production · 2026-07-09T02:17:51Z

 web-runner = { max-threads = 1 }

-[[profile.ci.overrides]]
+[[profile.default.overrides]]


_{⚪ LOW RISK}

Suggestion: Moving the web-runner group to profile.default.overrides ensures that tests booting full worker runtimes are always serialized. This is a significant stability improvement for local development, matching the deterministic behavior of the CI environment.

codecov · 2026-07-09T02:36:44Z

Codecov Report

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

Files with missing lines	Patch %	Lines
services/ws-web-runner/build.rs	20.00%	4 Missing ⚠️
services/ws-wasm-agent/src/lib.rs	57.14%	3 Missing ⚠️
services/ws-wasi-runner/src/host/mod.rs	95.00%	1 Missing ⚠️
services/ws-web-runner/src/runtime.rs	96.29%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

codacy-production

Pull Request Overview

This pull request successfully establishes a comprehensive multi-language coverage pipeline and addresses several intermittent CI failures. The implementation covers Rust, Python (via Pyodide), and browser-targeted WASM modules, satisfying all primary acceptance criteria. While the code is up to project standards according to Codacy, there is a medium-severity risk regarding coverage data corruption due to filename collisions in parallel test environments for WASI modules. Additionally, the refactoring of MSVC shims and JavaScript modules introduces several quality improvements, though some functions now exceed complexity limits.

About this PR

The scope of this PR is quite broad. In the future, consider separating infrastructure changes (coverage), bug fixes (CI flakiness), and refactoring (MSVC shims) into distinct pull requests to simplify the review process.
The PR description is empty. Providing context on the changes helps reviewers verify that all intended goals are met and understand the rationale behind specific architectural choices.

2 comments outside of the diff

services/ws-modules/pyface1/pkg/et_ws_pyface1.js

_{line 58 🟡 MEDIUM RISK}
Suggestion: The run method performs too many responsibilities: WebSocket initialization, camera media device setup, and ONNX session configuration. This results in a function length of 51 lines, exceeding the 50-line limit. Refactor by extracting the WebSocket connection logic (lines 70-79) and the MediaDevices setup (lines 80-87) into separate helper functions.

services/ws-web-runner/mingw-shim/msvc_crt_shim.c

_{line 1 ⚪ LOW RISK}
The block comment starting on line 1 contains the character sequence // (on line 10), which violates MISRA C:2012 Rule 3.1. This prohibits using comment markers within a comment block. Suggestion: change the inline comment marker to a description (e.g., '(double-slash) NOLINT').

Test suggestions

WASI guest modules write minicov profiles to the /cov preopen on completion
Pyodide modules capture coverage.py data and PUT it to the host storage bucket
The wasm-cov task successfully merges instrumented LLVM IR into the lcov report
int-wasm-cov-wrapper instruments only workspace crates for wasm32 targets
PyO3 runner tests successfully wait for peer registration under cold-start conditions

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

codacy-production · 2026-07-10T17:18:55Z

+    unsafe {
+        minicov::capture_coverage(&mut coverage).expect("minicov capture_coverage");
+    }
+    fs_err::write("/cov/et_ws_wasi_data1.profraw", coverage).expect("write /cov profraw");


_{🟡 MEDIUM RISK}

Hardcoded coverage filename causes collisions during parallel test runs. Use a unique suffix or the agent_id to ensure each test instance writes its own profile.

codacy-production · 2026-07-10T17:18:55Z

+    unsafe {
+        minicov::capture_coverage(&mut coverage).expect("minicov capture_coverage");
+    }
+    fs_err::write("/cov/et_ws_wasi_comm1.profraw", coverage).expect("write /cov profraw");


_{🟡 MEDIUM RISK}

Multiple tests running this module simultaneously will overwrite the same .profraw file in the shared coverage directory. Use a unique filename (e.g., incorporating a timestamp or agent_id) to avoid corruption during parallel test execution.

codacy-production · 2026-07-10T17:18:55Z

+#[case::speech_recognition("et-ws-speech-recognition", Language::Rust)]
+#[case::video1("et-ws-video1", Language::Rust)]
+#[case::pyface1("et-ws-pyface1", Language::Python)]
+fn hardware_module_load_fails(#[case] module: &str, #[case] language: Language) {


_{⚪ LOW RISK}

Suggestion: The hardware_module_load_fails test correctly exercises the entry-evaluation path of modules requiring unavailable browser APIs, ensuring these code paths contribute to the coverage report.

codacy-production · 2026-07-10T17:18:55Z

+pub fn parse_toml<T, P>(path: P, src: &str) -> Result<T, CliError>
 where
    T: for<'de> serde::Deserialize<'de>,
+    P: AsRef<Path>,
 {
    match toml::from_str(src) {


_{⚪ LOW RISK}

Nitpick: The implementation of parse_toml is identical in structure to parse_json (line 105). Consider using a generic helper function to reduce boilerplate and improve maintainability.

codacy-production · 2026-07-10T17:18:55Z

+// leading-underscore name is the CRT's, mandated by the archive's ABI, so the reserved-identifier check is
+// suppressed alongside it, as on the other shim symbols.
+// NOLINTNEXTLINE(clang-diagnostic-inconsistent-dllimport, bugprone-reserved-identifier, cert-dcl37-c)
+int _dupenv_s(char **buf, size_t *len, const char *name) {


_{⚪ LOW RISK}

Nitpick: The use of Win32 GetEnvironmentVariableA over stdlib getenv is a good security choice to avoid potential buffer over-reads.

jayvdb added 2 commits July 9, 2026 07:56

nextest

99b7c23

Use cargo-nextest for all test runs

34a7ed6

codacy-production Bot reviewed Jul 9, 2026

View reviewed changes

jayvdb added 15 commits July 9, 2026 12:59

clang-tidy the C shim

60b631d

more coverage

0852422

win fix

0755c64

another fix

eba99ee

more lint fixes

2f63766

codacy fixes

1e323ac

new rustc

80c9f03

backon timeout increase

8bca79d

fix et-path coverage

2d4f7e4

fix e2e-chrome

a70a0fc

Add wasm-cov-wrapper

58caa96

agent coverage

b45a26f

fix JS lints

17042e5

coverage feature

fa38580

fix .dockerignore

a85e4b6

jayvdb marked this pull request as ready for review July 10, 2026 17:17

jayvdb requested a review from pierre-tenedero July 10, 2026 17:17

codacy-production Bot reviewed Jul 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve coverage#76

Improve coverage#76
jayvdb wants to merge 17 commits into
mainfrom
nextest

jayvdb commented Jul 9, 2026

Uh oh!

deepsource-io Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

codacy-production Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

codacy-production Bot left a comment

Uh oh!

codacy-production Bot Jul 9, 2026

Uh oh!

codacy-production Bot Jul 9, 2026

Uh oh!

codecov Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

codacy-production Bot left a comment

Uh oh!

codacy-production Bot Jul 10, 2026

Uh oh!

codacy-production Bot Jul 10, 2026

Uh oh!

codacy-production Bot Jul 10, 2026

Uh oh!

codacy-production Bot Jul 10, 2026

Uh oh!

codacy-production Bot Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jayvdb commented Jul 9, 2026

Uh oh!

deepsource-io Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

codacy-production Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

About this PR

Test suggestions

Uh oh!

codacy-production Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

About this PR

Test suggestions

Uh oh!

codacy-production Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

deepsource-io Bot commented Jul 9, 2026 •

edited

Loading

codacy-production Bot commented Jul 9, 2026 •

edited

Loading

codecov Bot commented Jul 9, 2026 •

edited

Loading