[Code Health] Define types for ONNX Runtime session and state#223
[Code Health] Define types for ONNX Runtime session and state#223
Conversation
Replaces `any` typings with `InferenceSession` and `Tensor` from `onnxruntime-web`
in `SileroVAD.ts` to improve maintainability and strictly enforce correct ONNX type
boundaries. Also updates the global `ort` declaration to extract types via
`typeof import('onnxruntime-web')` natively.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Review Summary by QodoReplace any types with strict ONNX Runtime typings in SileroVAD
WalkthroughsDescription• Replace any types with strict InferenceSession and Tensor typings • Update global ort declaration to use native type extraction • Improve type safety and maintainability of VAD implementation • Enable TypeScript compiler protection for ONNX Runtime boundaries Diagramflowchart LR
A["SileroVAD.ts<br/>any types"] -->|"Replace with<br/>InferenceSession & Tensor"| B["SileroVAD.ts<br/>Strict types"]
C["ort: any"] -->|"Update to<br/>typeof import"| D["ort: typeof<br/>onnxruntime-web"]
B --> E["Better type safety<br/>& maintainability"]
D --> E
File Changes1. src/lib/vad/SileroVAD.ts
|
Code Review by Qodo
1. Missing onnxruntime-web dependency
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Rather than asserting
results.stateN as Tensor, consider givingresultsa more precise typed shape (e.g., a typedRunNamedOutputor custom interface) so the compiler tracks thestateNtype without needing a cast.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Rather than asserting `results.stateN as Tensor`, consider giving `results` a more precise typed shape (e.g., a typed `RunNamedOutput` or custom interface) so the compiler tracks the `stateN` type without needing a cast.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the type safety and maintainability of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively improves type safety in SileroVAD.ts by replacing any with strict types from onnxruntime-web, which is a great step for maintainability. I have two suggestions to further enhance the code quality: one regarding an unused class property and another to make a type cast safer with a runtime check.
| private stateC: any | null = null; | ||
| private srTensor: any | null = null; | ||
| private stateH: Tensor | null = null; | ||
| private stateC: Tensor | null = null; |
There was a problem hiding this comment.
The class property stateC is declared and typed, but it appears to be unused throughout the class. To improve code clarity and reduce the memory footprint, it should be removed.
|
|
||
| // Update persistent state | ||
| this.stateH = results.stateN; | ||
| this.stateH = results.stateN as Tensor; |
There was a problem hiding this comment.
Using as Tensor performs an unsafe type cast. While it satisfies the compiler, it offers no runtime safety and could lead to errors if the model's output format changes. A more robust approach is to perform a runtime type check using instanceof to validate that results.stateN is indeed a Tensor before assignment.
| this.stateH = results.stateN as Tensor; | |
| if (!(results.stateN instanceof ortLib.Tensor)) { | |
| throw new Error(`VAD model returned unexpected type for 'stateN'. Expected a Tensor.`); | |
| } | |
| this.stateH = results.stateN; |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| import type { InferenceSession, Tensor } from 'onnxruntime-web'; | ||
|
|
||
| // Use the globally-exposed ort from parakeet.js / onnxruntime-web | ||
| declare const ort: any; | ||
| declare const ort: typeof import('onnxruntime-web'); |
There was a problem hiding this comment.
1. Missing onnxruntime-web dependency 🐞 Bug ⛯ Reliability
SileroVAD.ts now requires TypeScript to resolve the onnxruntime-web module at compile time (via import type and typeof import(...)), but onnxruntime-web is not a direct dependency in this repo. In installs where parakeet.js's transitive dependencies are not hoisted to the root (common in strict/isolated setups), tsc will fail with "Cannot find module 'onnxruntime-web'" even though runtime still relies on a global ort.
Agent Prompt
### Issue description
`src/lib/vad/SileroVAD.ts` imports and type-queries `onnxruntime-web`, which requires the module to be resolvable at TypeScript compile time. The repo does not declare `onnxruntime-web` as a direct dependency, so builds can fail when the transitive dependency is not hoisted.
### Issue Context
Runtime still expects a globally-exposed `ort` (from `parakeet.js`), but compilation now depends on `onnxruntime-web` being resolvable.
### Fix Focus Areas
- package.json[1-40]
- src/lib/vad/SileroVAD.ts[1-6]
### Implementation notes
- Add `onnxruntime-web` to `dependencies` (preferred if the app expects it at runtime) or `devDependencies` (if truly only needed for types and guaranteed at runtime via global injection).
- Keep versions aligned with the expected runtime version (your README/vite config suggests `1.24.1`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ysdede
changed the title
1 participant
🎯 What: Replaced the use of
anytypes for the ONNX Runtime session and state variables with their strict types (InferenceSessionandTensor) fromonnxruntime-web.💡 Why: This improves the maintainability and type safety of
SileroVAD.ts. By leveraging the actual typings instead ofany, future changes or updates are properly protected by TypeScript compilation checks, reducing the risk of runtime errors.✅ Verification: Verified that
pnpm testruns cleanly and successfully, and that no unrequested dependencies were added topackage.json. The TypeScript compiler (tsc) was also run and confirmed thatorttypings are matched correctly.✨ Result: A more readable, predictable, and strongly-typed file with identical runtime behavior.
PR created automatically by Jules for task 7812390940331716981 started by @ysdede
Summary by Sourcery
Enhancements: