Add comprehensive tests for AudioSegmentProcessor#69
Conversation
This commit overwrites `src/lib/audio/AudioSegmentProcessor.test.ts` with a full test suite using `vitest`. The new tests cover: - Initialization and defaults - Noise floor adaptation (silence processing) - Speech onset detection - Speech offset detection with silence thresholds - Proactive segment splitting for long utterances - Statistics calculation - Configuration updates via setters This ensures the core VAD logic is reliable and behaves as expected. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThe test suite for AudioSegmentProcessor is significantly expanded, replacing a basic initialization test with comprehensive test coverage. New tests exercise initialization defaults, noise floor adaptation, speech onset/offset detection, segment splitting based on duration limits, statistical calculations, configuration changes, and state reset behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/lib/audio/AudioSegmentProcessor.test.ts`:
- Around line 64-65: The test captures the return value from
processor.processAudioData into the segments variable but never asserts on it;
add an assertion such as expect(segments).toHaveLength(0) (or appropriate
expected length) after calling processAudioData to confirm processAudioData (in
AudioSegmentProcessor.processAudioData) does not emit a segment prematurely.
- Around line 6-8: The test hardcodes windowSize (1280) which duplicates
DEFAULT_WINDOW_DURATION; update the test to import DEFAULT_WINDOW_DURATION (or
an exported computed WINDOW_SIZE) from the audio module and compute windowSize =
Math.round(DEFAULT_WINDOW_DURATION * sampleRate) so the test follows the
implementation; locate references to windowSize in AudioSegmentProcessor.test.ts
and replace the literal with the derived value using the imported
DEFAULT_WINDOW_DURATION (or use the module's exported computed value) so changes
to DEFAULT_WINDOW_DURATION automatically keep tests correct.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/audio/AudioSegmentProcessor.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/audio/AudioSegmentProcessor.test.ts (2)
src/lib/audio/AudioSegmentProcessor.ts (1)
AudioSegmentProcessor(102-609)src/lib/audio/audioParams.ts (1)
sampleRate(144-144)
🔇 Additional comments (7)
src/lib/audio/AudioSegmentProcessor.test.ts (7)
23-31: LGTM!Initialization test correctly verifies default state: stats defined, positive noise floor, no active speech.
33-49: LGTM!Good coverage of noise floor adaptation during sustained silence.
76-114: LGTM — well-documented silence threshold logic.The test clearly explains the chunk-by-chunk silence counter reasoning. Consider adding a few silence chunks before speech (as done in the onset test) to stabilize the noise floor, but the high energy delta (0.5 vs default 0.005) makes this unlikely to cause flakiness.
116-148: LGTM!Good test for proactive splitting. Verifies split occurs, segment duration meets minimum, and speech state persists after the split.
191-202: LGTM!Reset test properly verifies state is cleared and noise floor returns to the known default.
150-156: No action needed—maxSilenceWithinSpeechis a valid and fully implemented constructor option.The property is defined in the config interface, has a default value of 0.160 (160ms), and is actively used in the speech detection logic (line 250:
if (silenceDuration < this.options.maxSilenceWithinSpeech)). The test's use ofmaxSilenceWithinSpeech: 0correctly configures the processor to end speech immediately upon detecting any silence.Likely an incorrect or invalid review comment.
172-189: No issues found. Thereset()method only resets internal state (inSpeech, recentChunks, stats, etc.) and does not modify configuration stored inthis.options. ThesetThreshold()call on line 173 persists through thereset()call on line 181, so the test correctly expectsinSpeechto befalsewhen energy (0.5) is below the threshold (0.9).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const sampleRate = 16000; | ||
| // Default window duration is 0.08s (80ms), so 1280 samples at 16k | ||
| const windowSize = 1280; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded windowSize is fragile — derive from source constants.
windowSize is computed from an assumed DEFAULT_WINDOW_DURATION of 0.08s. If that constant changes in the implementation, these tests will silently pass wrong-sized chunks without failing, leading to subtle test inaccuracies. Consider importing DEFAULT_WINDOW_DURATION (or the computed windowSize) from the source module to keep the test in sync.
- const sampleRate = 16000;
- // Default window duration is 0.08s (80ms), so 1280 samples at 16k
- const windowSize = 1280;
+ const sampleRate = 16000;
+ // Import DEFAULT_WINDOW_DURATION from the source if exported,
+ // or keep this in sync with AudioSegmentProcessor's default.
+ const DEFAULT_WINDOW_DURATION = 0.08;
+ const windowSize = Math.round(DEFAULT_WINDOW_DURATION * sampleRate);Alternatively, if DEFAULT_WINDOW_DURATION is exported, import it directly.
#!/bin/bash
# Check if DEFAULT_WINDOW_DURATION is exported from the source
rg -n 'DEFAULT_WINDOW_DURATION' --type=ts -C2🤖 Prompt for AI Agents
In `@src/lib/audio/AudioSegmentProcessor.test.ts` around lines 6 - 8, The test
hardcodes windowSize (1280) which duplicates DEFAULT_WINDOW_DURATION; update the
test to import DEFAULT_WINDOW_DURATION (or an exported computed WINDOW_SIZE)
from the audio module and compute windowSize =
Math.round(DEFAULT_WINDOW_DURATION * sampleRate) so the test follows the
implementation; locate references to windowSize in AudioSegmentProcessor.test.ts
and replace the literal with the derived value using the imported
DEFAULT_WINDOW_DURATION (or use the module's exported computed value) so changes
to DEFAULT_WINDOW_DURATION automatically keep tests correct.
| const startTime = 10 * 0.08; | ||
| const segments = processor.processAudioData(chunk, startTime, 0.5); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused segments — consider asserting on it.
segments is captured but never checked. Adding expect(segments).toHaveLength(0) would strengthen the test by confirming that speech onset does not prematurely emit a segment.
🤖 Prompt for AI Agents
In `@src/lib/audio/AudioSegmentProcessor.test.ts` around lines 64 - 65, The test
captures the return value from processor.processAudioData into the segments
variable but never asserts on it; add an assertion such as
expect(segments).toHaveLength(0) (or appropriate expected length) after calling
processAudioData to confirm processAudioData (in
AudioSegmentProcessor.processAudioData) does not emit a segment prematurely.
Review Summary (Post r1→master merge)Status: ✅ MERGEABLE Changes: Comprehensive test overhaul for AudioSegmentProcessor. Replaces the existing minimal tests with proper structured tests covering:
Assessment: Solid improvement. Uses proper �eforeEach setup and helper functions (createSpeechChunk, createSilenceChunk). The tests are deterministic and well-structured. Note: Overlaps with PR #66 (both add long-segment splitting tests). If merging both, merge #66 first, then #69 will need minor conflict resolution in the test file. Recommendation: ✅ Merge — Significant test quality improvement |
Acknowledged. Thank you for the detailed review and the note regarding PR #66. |
This commit overwrites `src/lib/audio/AudioSegmentProcessor.test.ts` with a full test suite using `vitest`. The new tests cover: - Initialization and defaults - Noise floor adaptation (silence processing) - Speech onset detection - Speech offset detection with silence thresholds - Proactive segment splitting for long utterances - Statistics calculation - Configuration updates via setters This ensures the core VAD logic is reliable and behaves as expected. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
ysdede
changed the title
1 participant
Improved test coverage for AudioSegmentProcessor.ts by adding a comprehensive test suite in AudioSegmentProcessor.test.ts. Verified that tests pass with
bun testandnpm test.PR created automatically by Jules for task 4272967605191316370 started by @ysdede
Summary by CodeRabbit