docs: improve JSDoc coverage across keet public APIs#145
Conversation
Reviewer's GuideDocumentation-only update that standardizes Keet header version tags to v1.1 and adds/expands JSDoc/TSDoc comments for public types, component props, services, workers, and utility APIs across the app, without changing runtime behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Many of the new JSDoc descriptions duplicate the existing inline comments (e.g., in
AudioConfig,HasSpeechResult); consider removing redundant line comments to avoid drift and keep a single source of truth for field semantics. - You’ve standardized headers to
Keet v1.1even for files that previously declared higher major versions (e.g., v3/v4 workers and transcribers); if those version tags are intended to reflect feature generation rather than current app version, it may be worth double-checking that this downgrade won’t confuse consumers of these APIs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Many of the new JSDoc descriptions duplicate the existing inline comments (e.g., in `AudioConfig`, `HasSpeechResult`); consider removing redundant line comments to avoid drift and keep a single source of truth for field semantics.
- You’ve standardized headers to `Keet v1.1` even for files that previously declared higher major versions (e.g., v3/v4 workers and transcribers); if those version tags are intended to reflect feature generation rather than current app version, it may be worth double-checking that this downgrade won’t confuse consumers of these APIs.
## Individual Comments
### Comment 1
<location> `src/types/index.ts:80-81` </location>
<code_context>
startTime: number;
+ /** End timestamp in seconds. */
endTime: number;
+ /** Optional confidence score in the range 0-1. */
confidence: number;
}
</code_context>
<issue_to_address>
**issue:** Docstring marks `confidence` as optional, but the type is required.
The JSDoc marks `confidence` as optional but the type is required. If callers may omit this field, change it to `confidence?: number;`. If it must always be provided, update the JSDoc to remove "Optional" for consistency.
</issue_to_address>
### Comment 2
<location> `src/components/ModelLoadingOverlay.tsx:34-41` </location>
<code_context>
{ id: 'parakeet-tdt-0.6b-v3', name: 'Parakeet v3', desc: 'Multilingual Streaming' },
];
+/**
+ * Resolves a human-readable model label.
+ * @param id - Internal model ID.
+ * @returns Display name if known, otherwise the raw ID.
+ */
export function getModelDisplayName(id: string): string {
return (MODELS.find((m) => m.id === id)?.name ?? id) || 'Unknown model';
}
</code_context>
<issue_to_address>
**suggestion:** Behavior of `getModelDisplayName` can diverge from the documented fallback.
Because of `(… ?? id) || 'Unknown model'`, an empty-string `id` (or other falsy IDs) will return `'Unknown model'` instead of the raw ID. If you need the documented behavior, consider removing the `|| 'Unknown model'` or updating the JSDoc to mention the `'Unknown model'` fallback explicitly.
```suggestion
/**
* Resolves a human-readable model label.
* @param id - Internal model ID.
* @returns Display name if known, otherwise the raw ID.
*/
export function getModelDisplayName(id: string): string {
return MODELS.find((m) => m.id === id)?.name ?? id;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe PR introduces reactive signals for AudioEngine and MelWorkerClient accessibility, expands component and worker API surfaces with new optional props and type definitions, adds comprehensive JSDoc documentation across modules, and standardizes version headers from v2.0/v3.0/v4.0 to v1.1 throughout the codebase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/utils/time.ts (1)
9-10:⚠️ Potential issue | 🟡 Minor
sis not floored — floating-point seconds produce a malformed output string.
seconds % 60retains any fractional part.(30.5).toString()is"30.5", and.padStart(2, '0')only left-pads; it does not truncate. For an input of90.5the output is"01:30.5"instead of"01:30". Becausesecondsis typed asnumberand audio timestamps fromHTMLMediaElement.durationare routinely floats, this will manifest in real usage.🐛 Proposed fix
- const s = seconds % 60; + const s = Math.floor(seconds % 60);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/time.ts` around lines 9 - 10, The formatted time uses a fractional seconds value (const s = seconds % 60) which yields strings like "30.5"; fix by computing an integer totalSeconds = Math.floor(seconds) (or Math.trunc) and derive h/m/s from that so s is an integer before padding; update uses of seconds -> totalSeconds in the h/m/s calculations (look for the const s = ... and the return formatting expression) so formatted output becomes whole seconds.src/lib/transcription/transcription.worker.ts (1)
1-10:⚠️ Potential issue | 🟡 MinorDisambiguate the "Keet v1.1" header from the "v3/v4 mode" labels in the same block
After standardizing the header to
Keet v1.1, the adjacent comment block now places two unrelated versioning schemes side-by-side with no label to distinguish them:* Keet v1.1 - Transcription Web Worker ← application/package version * ... * - v3 token-stream mode … ← internal pipeline mode label * - v4 utterance mode … ← internal pipeline mode labelA reader unfamiliar with the project could infer that "v1.1" is simply an earlier generation than the "v3/v4" modes documented below it. A brief qualifier on the mode lines (e.g., "pipeline v3 token-stream mode …") would eliminate the ambiguity.
📝 Suggested wording
/** * Keet v1.1 - Transcription Web Worker * * Runs heavy AI inference and text merging in a background thread * to prevent UI stuttering on the main thread. * * Supports both: - * - v3 token-stream mode (LCSPTFAMerger, fixed-window) - * - v4 utterance mode (UtteranceBasedMerger, cursor-based windowing) + * - pipeline v3 token-stream mode (LCSPTFAMerger, fixed-window) + * - pipeline v4 utterance mode (UtteranceBasedMerger, cursor-based windowing) */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/transcription.worker.ts` around lines 1 - 10, Update the top-of-file comment in transcription.worker.ts to disambiguate the application version from internal pipeline modes: keep "Keet v1.1 - Transcription Web Worker" as the package/app header and change the two mode lines to explicitly label them as pipeline modes, e.g. "pipeline v3 token-stream mode (LCSPTFAMerger, fixed-window)" and "pipeline v4 utterance mode (UtteranceBasedMerger, cursor-based windowing)"; locate the block containing "Keet v1.1 - Transcription Web Worker" and the lines "v3 token-stream mode (LCSPTFAMerger, fixed-window)" and "v4 utterance mode (UtteranceBasedMerger, cursor-based windowing)" and edit those lines only to add the qualifier.src/lib/transcription/SentenceBoundaryDetector.ts (1)
423-432:⚠️ Potential issue | 🟠 MajorHash collision in
generateCacheKeycan silently return wrong cached sentence boundaries.A 32-bit polynomial hash has a non-trivial collision probability for real-world transcription texts (birthday paradox: ~50 % chance of at least one collision after ~65 000 distinct cache keys). When two different texts hash to the same key,
performNLPreturns the cached result of the first text for the second without any detection. The bug is silent — no error is raised and the caller receives incorrect sentence boundaries.Fix: either append the text length to the key (cheap partial disambiguation), use a cryptographic hash (e.g. a
SubtleCryptoSHA-256 digest — async but collision-resistant), or simply store the original text as the cache key (usually fine givencacheSize: 100default and typical sentence-level granularity).🔑 Minimal fix — key by length-prefixed hash
private generateCacheKey(text: string): string { let hash = 0; - if (text.length === 0) return hash.toString(); + if (text.length === 0) return '0:0'; for (let i = 0; i < text.length; i++) { const char = text.charCodeAt(i); hash = ((hash << 5) - hash) + char; hash = hash & hash; } - return hash.toString(); + return `${text.length}:${hash >>> 0}`; // include length as cheap disambiguator }Or, for a small cache (≤100 entries), use the text itself as the key:
private generateCacheKey(text: string): string { - let hash = 0; - if (text.length === 0) return hash.toString(); - for (let i = 0; i < text.length; i++) { - const char = text.charCodeAt(i); - hash = ((hash << 5) - hash) + char; - hash = hash & hash; - } - return hash.toString(); + return text; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/SentenceBoundaryDetector.ts` around lines 423 - 432, The current generateCacheKey implementation can collide and cause performNLP to return wrong cached boundaries; replace it so the cache key is unambiguous (e.g., use the full input text as the key or at minimum prefix the 32-bit hash with the text length) and update any references to the cache lookup accordingly (see generateCacheKey and where performNLP accesses the cache/Map). For the small default cacheSize (100) using the raw text as the key is simplest and safest—change generateCacheKey to return the input text (or "len:hash" if you prefer a compact key) so cached entries map uniquely to their original text.
🧹 Nitpick comments (8)
src/lib/transcription/ModelManager.ts (1)
236-237: Import formatting is inconsistent with the identical import on line 100.Line 100 keeps the import on a single line; the reformatted version places the module string on a new line but attaches the closing
)to it, which is an unusual style. Consider either reverting to one line (matching line 100) or placing the closing)on its own line.♻️ Align with line 100 style
- const { ParakeetModel } = await import( - 'parakeet.js'); + const { ParakeetModel } = await import('parakeet.js');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/ModelManager.ts` around lines 236 - 237, The dynamic import of ParakeetModel in ModelManager.ts is formatted differently from the identical import at line 100; change the import to match line 100 by putting it on a single line (e.g., const { ParakeetModel } = await import('parakeet.js');) so the style is consistent, or alternatively move the closing parenthesis to its own line if you prefer multiline—update the import expression that references ParakeetModel to follow the same formatting as the earlier import.src/utils/time.ts (1)
6-10: Consider guarding against negative and non-finite inputs.A negative or
NaN/Infinityvalue produces garbage output (e.g.,Math.floor(-0.001)=-1). For a public duration-formatting utility, a short guard makes the contract explicit and prevents silent mis-rendering.🛡️ Proposed guard
export function formatDuration(seconds: number): string { + if (!isFinite(seconds) || seconds < 0) return '00:00'; const h = Math.floor(seconds / 3600);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/time.ts` around lines 6 - 10, The formatDuration function currently assumes seconds is a non-negative finite number; add a guard at the start of formatDuration to validate the input (check typeof seconds === 'number' and Number.isFinite(seconds') and treat negative values or non-finite/NaN as 0 or return a clear default string (e.g., "00:00" or "0:00:00" depending on existing behavior). Update formatDuration to clamp seconds to Math.max(0, Math.floor(seconds)) after validation so h/m/s calculations never see negative or non-finite values; keep the rest of the formatting logic unchanged.src/lib/transcription/WindowBuilder.ts (2)
85-148: Optional: add@param/@returnstags to public methods.All methods have a prose description, but none carry
@paramor@returnstags. For a documentation-coverage PR, adding them for the non-trivial public methods would give IDE tooling (hover hints, signature help) their full value.Example for
markSentenceEndand the cursor helpers:📝 Example `@param` / `@returns` additions
- /** - * Record the end frame of a fully finalized sentence. - */ - markSentenceEnd(frameIdx: number): void { + /** + * Record the end frame of a fully finalized sentence. + * `@param` frameIdx - Global frame index of the sentence boundary. + */ + markSentenceEnd(frameIdx: number): void {- /** - * Advance the mature cursor to a finalized sentence boundary. - * The mature cursor marks where transcription is considered stable. - */ - advanceMatureCursor(frameIdx: number): void { + /** + * Advance the mature cursor to a finalized sentence boundary. + * The mature cursor marks where transcription is considered stable. + * `@param` frameIdx - Global frame index to advance the cursor to; ignored if not strictly greater than the current position. + */ + advanceMatureCursor(frameIdx: number): void {- /** - * Advance the mature cursor using a time value (seconds). - * Converts to frame offset based on sample rate. - */ - advanceMatureCursorByTime(timeSec: number): void { + /** + * Advance the mature cursor using a time value (seconds). + * Converts to frame offset based on sample rate. + * `@param` timeSec - Time in seconds; converted to a frame index via `Math.round(timeSec * sampleRate)`. + */ + advanceMatureCursorByTime(timeSec: number): void {- /** - * Get current mature cursor position in frames. - */ - getMatureCursorFrame(): number { + /** + * Get current mature cursor position in frames. + * `@returns` Global frame index of the mature cursor (0 until first sentence). + */ + getMatureCursorFrame(): number {- /** - * Get current mature cursor position in seconds. - */ - getMatureCursorTime(): number { + /** + * Get current mature cursor position in seconds. + * `@returns` Cursor position in seconds (0 until first sentence). + */ + getMatureCursorTime(): number {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/WindowBuilder.ts` around lines 85 - 148, Add JSDoc `@param` and `@returns` tags to the public methods markSentenceEnd, advanceMatureCursor, advanceMatureCursorByTime, getMatureCursorFrame, and getMatureCursorTime: document the parameter names and types (e.g., frameIdx: number, timeSec: number) with `@param` and document return types for the getters and advanceMatureCursorByTime using `@returns` (e.g., number or void) so IDE hover/signature help shows explicit param and return info; keep the existing prose summary and debug notes, only augment each method's comment block with the appropriate `@param/`@returns annotations.
62-81: Optional: document the constructor parameters.Since the PR is specifically targeting JSDoc coverage, the constructor is the one public-facing entry point still missing documentation. The three parameters (
ringBuffer,vadBuffer,config) have non-obvious semantics (e.g.,vadBuffer = nulldisables VAD entirely).📝 Proposed constructor JSDoc
+ /** + * `@param` ringBuffer - The audio ring buffer supplying frame data. + * `@param` vadBuffer - Optional VAD ring buffer; pass `null` to disable VAD boundary refinement. + * `@param` config - Partial configuration overrides; unset keys fall back to built-in defaults. + */ constructor( ringBuffer: IRingBuffer, vadBuffer: VADRingBuffer | null = null, config: Partial<WindowBuilderConfig> = {} ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/WindowBuilder.ts` around lines 62 - 81, Add a JSDoc block to the WindowBuilder constructor documenting each parameter and the defaults/semantics: describe ringBuffer (IRingBuffer) as required input audio buffer, vadBuffer (VADRingBuffer | null) where null disables VAD behavior, and config (Partial<WindowBuilderConfig>) with its default values and meanings (sampleRate, minDurationSec, maxDurationSec, minInitialDurationSec, maxSentences, useVadBoundaries, vadSilenceThreshold, debug). Mention that config overrides defaults and that useVadBoundaries=true depends on a non-null vadBuffer, and note any units (seconds) for duration fields to clarify usage; attach this JSDoc immediately above the constructor declaration.src/types/index.ts (1)
61-70:TranscriptionResultinsrc/types/index.tsis unused dead code and should be removed or its purpose clarified.This file defines
TranscriptionResultwith fieldstext,tokens: Token[],isFinal, andtimestamp, but no code in the repository imports it from here. The actual runtime code importsTranscriptionResultfrom eithersrc/lib/transcription/types.ts(chunkText,text,words,totalDuration,isFinal) orsrc/lib/model/types.ts(text,words?). Sincesrc/types/index.tsappears to serve as a public API facade (evidenced byappStore.tsimportingRecordingState,ModelState, andBackendTypefrom there), either remove this unused definition or confirm it represents an intended public API and align it with the actual runtime interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/index.ts` around lines 61 - 70, Remove or reconcile the unused TranscriptionResult type: either delete the TranscriptionResult interface (and any unused Token type) from src/types/index.ts if it's dead, or update it to match the actual runtime shape used elsewhere (e.g., align fields to chunkText/text/words/totalDuration/isFinal as in lib/transcription/types.ts and optional words as in lib/model/types.ts) and then export that canonical version so other modules import the correct public API; update any re-exports in the module to ensure RecordingState/ModelState/BackendType exports remain intact and run a repo-wide search for TranscriptionResult to confirm no stale imports remain.src/components/ContextPanel.tsx (1)
14-15: New optional proponDeviceSelectgoes beyond documentation.The PR description states "documentation-only; no runtime behavior changes intended," but this adds a new callback prop to
ContextPanelProps. While it's optional and won't break existing callers, it is a public API surface change — worth noting in the PR description for accuracy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ContextPanel.tsx` around lines 14 - 15, The PR added a new optional prop onDeviceSelect to ContextPanelProps in ContextPanel.tsx which is a public API surface change; update the PR description to explicitly mention this new callback prop (onDeviceSelect) and that it was added to ContextPanelProps so reviewers/users are aware of the API change (or alternatively mark it internal/undocumented if it wasn’t intended as a public API change).src/lib/transcription/SentenceBoundaryDetector.ts (2)
287-301: O(n²) in heuristic path — drop the superfluousindexOf.The
_idxparameter in.map((word, _idx) => …)is deliberately discarded, thenwords.indexOf(word)does a full O(n) linear scan for every filtered word. The index is already available from a singlereduce/flatMappass over the original array.♻️ O(n) rewrite
private detectSentenceEndingsHeuristic(words: DetectorWord[]): SentenceEndingWord[] { - return words - .filter(word => /[.?!]$/.test(word.text)) - .map((word, _idx) => { - const wordIndex = words.indexOf(word); - return { - ...word, - wordIndex, - sentenceMetadata: { - sentenceText: word.text, - detectionMethod: 'heuristic' as const, - }, - }; - }); + return words.reduce<SentenceEndingWord[]>((acc, word, wordIndex) => { + if (/[.?!]$/.test(word.text)) { + acc.push({ + ...word, + wordIndex, + sentenceMetadata: { + sentenceText: word.text, + detectionMethod: 'heuristic' as const, + }, + }); + } + return acc; + }, []); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/SentenceBoundaryDetector.ts` around lines 287 - 301, The heuristic path in detectSentenceEndingsHeuristic currently calls words.indexOf(word) inside a .map, causing O(n²); instead preserve the original index by iterating with the index from the source array (e.g., use words.reduce or words.forEach or words.flatMap with the (word, idx) callback) and when you detect /[.?!]$/.test(word.text) push the SentenceEndingWord with wordIndex set to idx and sentenceMetadata set as before; update detectSentenceEndingsHeuristic to return a SentenceEndingWord[] built in O(n) without using indexOf.
65-65: ImportWinkMethodsfromwink-nlpand use it for proper type safety.Replace
private nlp: any | null = null;withprivate nlp: WinkMethods | null = null;by addingWinkMethodsto the import statement:import winkNLP, { type WinkMethods } from 'wink-nlp';Using
anyloses all type safety for everythis.nlp.readDoc()andthis.nlp.sentences()call site.winkNLP()returns a properly typedWinkMethodsobject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/transcription/SentenceBoundaryDetector.ts` at line 65, Update the SentenceBoundaryDetector class to use the proper Wink NLP types: change the type of the private field nlp from any to WinkMethods (i.e., replace "private nlp: any | null = null" with "private nlp: WinkMethods | null = null") and add the type import to the top-level import for wink-nlp (import winkNLP, { type WinkMethods } from 'wink-nlp'); this will give correct typing for calls like this.nlp.readDoc() and this.nlp.sentences().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Sidebar.tsx`:
- Around line 4-25: The SidebarProps type declares audioLevel and the Sidebar
component never reads or uses props.audioLevel; either remove audioLevel from
SidebarProps or surface it in the UI—e.g., read audioLevel inside the Sidebar
component (where onToggleRecording/RecordButton and the record button area are
implemented) and render a simple level indicator or pass it down to the existing
RecordButton/record area so the prop is actually consumed; update SidebarProps
and any callsites accordingly to keep types consistent (modify
SidebarProps.audioLevel or the RecordButton props depending on where you wire
the value).
In `@src/lib/transcription/ModelManager.ts`:
- Line 2: Update the stale "v2.0" version reference in the internal comment
inside ModelManager (the comment mentioning "parakeet.js/IndexedDB cache") so it
matches the file header "Keet v1.1" or remove the version token entirely; locate
the comment within the ModelManager class (the block that begins "// In v2.0 we
rely on parakeet.js/IndexedDB cache, but we can do a quick check") and either
change "v2.0" to "v1.1" or rewrite it to avoid a specific version mention while
preserving the intent about checking the parakeet.js/IndexedDB cache.
In `@src/lib/transcription/types.ts`:
- Around line 5-8: The BackendType and ModelState type definitions are
duplicated; move the single source-of-truth for these types into a canonical
shared types module (the central types index module), export BackendType and
ModelState from there, remove the duplicate type declarations in transcription
types and model types and the inline literal types inside ModelLoadingOverlay,
and update those modules to import/re-export the canonical types (ensure names
remain BackendType and ModelState so references compile).
---
Outside diff comments:
In `@src/lib/transcription/SentenceBoundaryDetector.ts`:
- Around line 423-432: The current generateCacheKey implementation can collide
and cause performNLP to return wrong cached boundaries; replace it so the cache
key is unambiguous (e.g., use the full input text as the key or at minimum
prefix the 32-bit hash with the text length) and update any references to the
cache lookup accordingly (see generateCacheKey and where performNLP accesses the
cache/Map). For the small default cacheSize (100) using the raw text as the key
is simplest and safest—change generateCacheKey to return the input text (or
"len:hash" if you prefer a compact key) so cached entries map uniquely to their
original text.
In `@src/lib/transcription/transcription.worker.ts`:
- Around line 1-10: Update the top-of-file comment in transcription.worker.ts to
disambiguate the application version from internal pipeline modes: keep "Keet
v1.1 - Transcription Web Worker" as the package/app header and change the two
mode lines to explicitly label them as pipeline modes, e.g. "pipeline v3
token-stream mode (LCSPTFAMerger, fixed-window)" and "pipeline v4 utterance mode
(UtteranceBasedMerger, cursor-based windowing)"; locate the block containing
"Keet v1.1 - Transcription Web Worker" and the lines "v3 token-stream mode
(LCSPTFAMerger, fixed-window)" and "v4 utterance mode (UtteranceBasedMerger,
cursor-based windowing)" and edit those lines only to add the qualifier.
In `@src/utils/time.ts`:
- Around line 9-10: The formatted time uses a fractional seconds value (const s
= seconds % 60) which yields strings like "30.5"; fix by computing an integer
totalSeconds = Math.floor(seconds) (or Math.trunc) and derive h/m/s from that so
s is an integer before padding; update uses of seconds -> totalSeconds in the
h/m/s calculations (look for the const s = ... and the return formatting
expression) so formatted output becomes whole seconds.
---
Nitpick comments:
In `@src/components/ContextPanel.tsx`:
- Around line 14-15: The PR added a new optional prop onDeviceSelect to
ContextPanelProps in ContextPanel.tsx which is a public API surface change;
update the PR description to explicitly mention this new callback prop
(onDeviceSelect) and that it was added to ContextPanelProps so reviewers/users
are aware of the API change (or alternatively mark it internal/undocumented if
it wasn’t intended as a public API change).
In `@src/lib/transcription/ModelManager.ts`:
- Around line 236-237: The dynamic import of ParakeetModel in ModelManager.ts is
formatted differently from the identical import at line 100; change the import
to match line 100 by putting it on a single line (e.g., const { ParakeetModel }
= await import('parakeet.js');) so the style is consistent, or alternatively
move the closing parenthesis to its own line if you prefer multiline—update the
import expression that references ParakeetModel to follow the same formatting as
the earlier import.
In `@src/lib/transcription/SentenceBoundaryDetector.ts`:
- Around line 287-301: The heuristic path in detectSentenceEndingsHeuristic
currently calls words.indexOf(word) inside a .map, causing O(n²); instead
preserve the original index by iterating with the index from the source array
(e.g., use words.reduce or words.forEach or words.flatMap with the (word, idx)
callback) and when you detect /[.?!]$/.test(word.text) push the
SentenceEndingWord with wordIndex set to idx and sentenceMetadata set as before;
update detectSentenceEndingsHeuristic to return a SentenceEndingWord[] built in
O(n) without using indexOf.
- Line 65: Update the SentenceBoundaryDetector class to use the proper Wink NLP
types: change the type of the private field nlp from any to WinkMethods (i.e.,
replace "private nlp: any | null = null" with "private nlp: WinkMethods | null =
null") and add the type import to the top-level import for wink-nlp (import
winkNLP, { type WinkMethods } from 'wink-nlp'); this will give correct typing
for calls like this.nlp.readDoc() and this.nlp.sentences().
In `@src/lib/transcription/WindowBuilder.ts`:
- Around line 85-148: Add JSDoc `@param` and `@returns` tags to the public methods
markSentenceEnd, advanceMatureCursor, advanceMatureCursorByTime,
getMatureCursorFrame, and getMatureCursorTime: document the parameter names and
types (e.g., frameIdx: number, timeSec: number) with `@param` and document return
types for the getters and advanceMatureCursorByTime using `@returns` (e.g., number
or void) so IDE hover/signature help shows explicit param and return info; keep
the existing prose summary and debug notes, only augment each method's comment
block with the appropriate `@param/`@returns annotations.
- Around line 62-81: Add a JSDoc block to the WindowBuilder constructor
documenting each parameter and the defaults/semantics: describe ringBuffer
(IRingBuffer) as required input audio buffer, vadBuffer (VADRingBuffer | null)
where null disables VAD behavior, and config (Partial<WindowBuilderConfig>) with
its default values and meanings (sampleRate, minDurationSec, maxDurationSec,
minInitialDurationSec, maxSentences, useVadBoundaries, vadSilenceThreshold,
debug). Mention that config overrides defaults and that useVadBoundaries=true
depends on a non-null vadBuffer, and note any units (seconds) for duration
fields to clarify usage; attach this JSDoc immediately above the constructor
declaration.
In `@src/types/index.ts`:
- Around line 61-70: Remove or reconcile the unused TranscriptionResult type:
either delete the TranscriptionResult interface (and any unused Token type) from
src/types/index.ts if it's dead, or update it to match the actual runtime shape
used elsewhere (e.g., align fields to chunkText/text/words/totalDuration/isFinal
as in lib/transcription/types.ts and optional words as in lib/model/types.ts)
and then export that canonical version so other modules import the correct
public API; update any re-exports in the module to ensure
RecordingState/ModelState/BackendType exports remain intact and run a repo-wide
search for TranscriptionResult to confirm no stale imports remain.
In `@src/utils/time.ts`:
- Around line 6-10: The formatDuration function currently assumes seconds is a
non-negative finite number; add a guard at the start of formatDuration to
validate the input (check typeof seconds === 'number' and
Number.isFinite(seconds') and treat negative values or non-finite/NaN as 0 or
return a clear default string (e.g., "00:00" or "0:00:00" depending on existing
behavior). Update formatDuration to clamp seconds to Math.max(0,
Math.floor(seconds)) after validation so h/m/s calculations never see negative
or non-finite values; keep the rest of the formatting logic unchanged.
| /** Currently selected sidebar tab ID. */ | ||
| activeTab: string; | ||
| /** Switches the selected sidebar tab. */ | ||
| onTabChange: (tab: string) => void; | ||
| // Recording controls | ||
| /** Indicates whether recording is active. */ | ||
| isRecording: boolean; | ||
| /** Starts or stops audio capture. */ | ||
| onToggleRecording: () => void; | ||
| // Model state | ||
| /** Whether a model is loaded and ready for inference. */ | ||
| isModelReady: boolean; | ||
| /** Opens model selection/loading flow. */ | ||
| onLoadModel: () => void; | ||
| /** Current model lifecycle state. */ | ||
| modelState: string; | ||
| // Device selection | ||
| /** Available microphone devices from `enumerateDevices()`. */ | ||
| availableDevices: MediaDeviceInfo[]; | ||
| /** Selected microphone device ID. */ | ||
| selectedDeviceId: string; | ||
| /** Updates the selected microphone device. */ | ||
| onDeviceSelect: (id: string) => void; | ||
| // Audio feedback | ||
| /** Current normalized input level (0-1). */ | ||
| audioLevel: number; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's search for all audioLevel references in the codebase
rg -n "audioLevel" --type tsx --type ts -B 2 -A 2Repository: ysdede/keet
Length of output: 82
🏁 Script executed:
# Also let's examine the actual Sidebar.tsx file to verify the structure
wc -l src/components/Sidebar.tsxRepository: ysdede/keet
Length of output: 85
🏁 Script executed:
# Read the Sidebar component to see the actual usage
cat -n src/components/Sidebar.tsx | head -80Repository: ysdede/keet
Length of output: 4119
🏁 Script executed:
# Read the rest of the Sidebar component
cat -n src/components/Sidebar.tsx | tail -80Repository: ysdede/keet
Length of output: 4206
🏁 Script executed:
# Search for audioLevel using simple grep (works regardless of file extension)
grep -n "audioLevel" src/components/Sidebar.tsxRepository: ysdede/keet
Length of output: 79
🏁 Script executed:
# Also check if audioLevel is referenced in any other files
grep -r "audioLevel" src/ --include="*.tsx" --include="*.ts" --include="*.jsx" --include="*.js"Repository: ysdede/keet
Length of output: 942
audioLevel prop is declared and documented but unused in the Sidebar component.
The new JSDoc correctly describes the prop's intent, but props.audioLevel is never read or forwarded to child components anywhere in the component body (lines 52–146). Documenting a dead prop makes the discrepancy more visible and could mislead callers into thinking it's wired into the UI.
Either remove the prop from SidebarProps since Sidebar doesn't use it, or integrate it into the UI (e.g., a level indicator in the record button area).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Sidebar.tsx` around lines 4 - 25, The SidebarProps type
declares audioLevel and the Sidebar component never reads or uses
props.audioLevel; either remove audioLevel from SidebarProps or surface it in
the UI—e.g., read audioLevel inside the Sidebar component (where
onToggleRecording/RecordButton and the record button area are implemented) and
render a simple level indicator or pass it down to the existing
RecordButton/record area so the prop is actually consumed; update SidebarProps
and any callsites accordingly to keep types consistent (modify
SidebarProps.audioLevel or the RecordButton props depending on where you wire
the value).
There was a problem hiding this comment.
Issue filed #152. Will address Sidebar audioLevel usage or removal in SidebarProps. Follow-up work; does not block merge.
There was a problem hiding this comment.
@ysdede, sounds good! Issue #152 will track the audioLevel integration or removal. Thanks for filing it—happy to help review when you address it in the follow-up PR. 🚀
| @@ -1,5 +1,5 @@ | |||
| /** | |||
| * Keet v2.0 - Model Manager | |||
| * Keet v1.1 - Model Manager | |||
There was a problem hiding this comment.
Stale v2.0 reference at line 67 should be updated alongside the header.
The header now reads Keet v1.1, but line 67 retains:
// In v2.0 we rely on parakeet.js/IndexedDB cache, but we can do a quick check
This internal comment contradicts the new version tag and may mislead future maintainers. Update or remove the version reference in that comment to stay consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/transcription/ModelManager.ts` at line 2, Update the stale "v2.0"
version reference in the internal comment inside ModelManager (the comment
mentioning "parakeet.js/IndexedDB cache") so it matches the file header "Keet
v1.1" or remove the version token entirely; locate the comment within the
ModelManager class (the block that begins "// In v2.0 we rely on
parakeet.js/IndexedDB cache, but we can do a quick check") and either change
"v2.0" to "v1.1" or rewrite it to avoid a specific version mention while
preserving the intent about checking the parakeet.js/IndexedDB cache.
There was a problem hiding this comment.
Issue filed #154. Will update stale v2.0 comment to v1.1 or remove version reference in ModelManager. Follow-up work; does not block merge.
There was a problem hiding this comment.
@ysdede, thank you for filing issue #154 to track this! Sounds like a good plan to address it in the follow-up. 👍
| /** Model loading lifecycle state. */ | ||
| export type ModelState = 'unloaded' | 'loading' | 'ready' | 'error'; | ||
| /** Inference backend name. */ | ||
| export type BackendType = 'webgpu' | 'wasm'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all definitions of ModelState and BackendType across the codebase
echo "=== ModelState definitions ==="
rg -n "export type ModelState\b" --type=ts
echo ""
echo "=== BackendType definitions ==="
rg -n "export type BackendType\b" --type=ts
echo ""
echo "=== Inline literal duplicates of these union types ==="
echo "--- Pattern: 'unloaded' | 'loading' | 'ready' | 'error' ---"
rg -nP "'unloaded'\s*\|\s*'loading'\s*\|\s*'ready'\s*\|\s*'error'" --type=ts
echo ""
echo "--- Pattern: 'webgpu' | 'wasm' ---"
rg -nP "'webgpu'\s*\|\s*'wasm'" --type=tsRepository: ysdede/keet
Length of output: 1261
BackendType is duplicated across three files; ModelState across two. Consolidate to a single canonical location.
BackendType is defined in src/types/index.ts (line 100), src/lib/model/types.ts (line 14), and src/lib/transcription/types.ts (line 8). ModelState is defined in src/types/index.ts (line 98) and src/lib/transcription/types.ts (line 6). Additionally, ModelLoadingOverlay.tsx duplicates these types as inline literals. Define these types in a single canonical location and re-export where needed to prevent drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/transcription/types.ts` around lines 5 - 8, The BackendType and
ModelState type definitions are duplicated; move the single source-of-truth for
these types into a canonical shared types module (the central types index
module), export BackendType and ModelState from there, remove the duplicate type
declarations in transcription types and model types and the inline literal types
inside ModelLoadingOverlay, and update those modules to import/re-export the
canonical types (ensure names remain BackendType and ModelState so references
compile).
There was a problem hiding this comment.
Issue filed #148. Will consolidate BackendType and ModelState to single source of truth in central types. Follow-up work; does not block merge.
There was a problem hiding this comment.
@ysdede Thank you for filing issue #148! 🎯 Deferring the type consolidation to a follow-up PR is perfectly reasonable—this keeps the documentation-focused scope of this PR clean while ensuring the duplication is tracked for resolution.
/)/)
(•.•)
/>🎫 Issue filed!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
docs: improve JSDoc coverage across keet public APIs e940a08
Summary
Keet v1.1.Validation
npm testnpm run buildSummary by Sourcery
Improve documentation coverage and consistency across public transcription APIs and UI components.
Documentation
v1.1version tag across core modules.Summary by CodeRabbit
Release Notes
New Features
Documentation