CodeRabbit Generated Unit Tests: Generate unit tests for PR changes

[coderabbitai]

Unit test generation was requested by @ysdede.

• Optimize Incremental Mel Calculation #29 (comment)

The following files were modified:

• tests/mel-extended.test.mjs

Summary by CodeRabbit

• Tests
  • Added comprehensive test suite covering edge cases, boundary conditions, incremental processing scenarios, and streaming behavior to validate numerical stability and reliability across various input conditions.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@tests/mel-extended.test.mjs`:
- Around line 636-639: The test contains a tautological assertion: replace the
check of features length to validate it's equal to nMels times the returned
frame count; specifically, in the test that destructures { features, length }
from preprocessor.process(audio) and uses nMels, change the assertion from
expecting features.length to equal nMels * (features.length / nMels) to
expecting features.length toBe(nMels * length) so it actually verifies features
were produced for each mel bin per frame.
- Around line 192-207: The test currently guards assertions with a conditional
on featuresLen, which can silently skip assertions; replace the conditional
check with an explicit assertion using expect(featuresLen).toBe(1) right after
calling preprocessor.computeRawMel(audio), and then always call
preprocessor.normalizeFeatures(rawMel, nFrames, featuresLen) and run the
zero-checks on normalized so the test fails loudly when preconditions
(featuresLen === 1) aren't met; reference the variables/featuresLen and the
functions preprocessor.computeRawMel and preprocessor.normalizeFeatures when
making the change.

🧹 Nitpick comments (2)

tests/mel-extended.test.mjs (2)

369-378: Unused variables nFramesFull and nFramesInc.

These variables are declared but never used in the comparison loop. Either remove them or use them if they were intended for a different iteration pattern.

♻️ Proposed fix

       // Compare features
-      const nFramesFull = fullResult.features.length / 128;
-      const nFramesInc = incResult.features.length / 128;
       let maxErr = 0;
       for (let i = 0; i < Math.min(fullResult.features.length, incResult.features.length); i++) {
         const err = Math.abs(fullResult.features[i] - incResult.features[i]);
         if (err > maxErr) maxErr = err;
       }

---

648-657: Consider a more specific assertion.

The assertion toBeGreaterThanOrEqual(0) is quite weak. If the expected behavior for audio of length N_FFT is well-defined, consider asserting the exact expected frame count.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Test assertions may be silently skipped when featuresLen !== 1.

The conditional if (featuresLen === 1) means the assertions might not execute if the audio length doesn't produce exactly one frame. Consider using expect(featuresLen).toBe(1) to fail explicitly if the precondition isn't met, or adjust the audio length to guarantee one frame.

🔧 Proposed fix

   it('should handle single frame (featuresLen=1) by outputting zeros', () => {
     const audio = new Float32Array(160); // Exactly 1 frame worth of samples
     for (let i = 0; i < audio.length; i++) {
       audio[i] = Math.sin(2 * Math.PI * 440 * i / 16000);
     }

     const { rawMel, nFrames, featuresLen } = preprocessor.computeRawMel(audio);
-    if (featuresLen === 1) {
-      const normalized = preprocessor.normalizeFeatures(rawMel, nFrames, featuresLen);
-
-      // With single frame, normalization should output zeros (invStd = 0)
-      for (let i = 0; i < normalized.length; i++) {
-        expect(normalized[i]).toBe(0);
-      }
-    }
+    expect(featuresLen).toBe(1); // Ensure precondition is met
+    const normalized = preprocessor.normalizeFeatures(rawMel, nFrames, featuresLen);
+
+    // With single frame, normalization should output zeros (invStd = 0)
+    for (let i = 0; i < normalized.length; i++) {
+      expect(normalized[i]).toBe(0);
+    }
   });

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	it('should handle single frame (featuresLen=1) by outputting zeros', () => {
	const audio = new Float32Array(160); // Exactly 1 frame worth of samples
	for (let i = 0; i < audio.length; i++) {
	audio[i] = Math.sin(2 * Math.PI * 440 * i / 16000);
	}
	const { rawMel, nFrames, featuresLen } = preprocessor.computeRawMel(audio);
	if (featuresLen === 1) {
	const normalized = preprocessor.normalizeFeatures(rawMel, nFrames, featuresLen);
	// With single frame, normalization should output zeros (invStd = 0)
	for (let i = 0; i < normalized.length; i++) {
	expect(normalized[i]).toBe(0);
	}
	}
	});
	it('should handle single frame (featuresLen=1) by outputting zeros', () => {
	const audio = new Float32Array(160); // Exactly 1 frame worth of samples
	for (let i = 0; i < audio.length; i++) {
	audio[i] = Math.sin(2 * Math.PI * 440 * i / 16000);
	}
	const { rawMel, nFrames, featuresLen } = preprocessor.computeRawMel(audio);
	expect(featuresLen).toBe(1); // Ensure precondition is met
	const normalized = preprocessor.normalizeFeatures(rawMel, nFrames, featuresLen);
	// With single frame, normalization should output zeros (invStd = 0)
	for (let i = 0; i < normalized.length; i++) {
	expect(normalized[i]).toBe(0);
	}
	});

🤖 Prompt for AI Agents

In `@tests/mel-extended.test.mjs` around lines 192 - 207, The test currently
guards assertions with a conditional on featuresLen, which can silently skip
assertions; replace the conditional check with an explicit assertion using
expect(featuresLen).toBe(1) right after calling
preprocessor.computeRawMel(audio), and then always call
preprocessor.normalizeFeatures(rawMel, nFrames, featuresLen) and run the
zero-checks on normalized so the test fails loudly when preconditions
(featuresLen === 1) aren't met; reference the variables/featuresLen and the
functions preprocessor.computeRawMel and preprocessor.normalizeFeatures when
making the change.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Tautological assertion always passes.

The assertion expect(features.length).toBe(nMels * (features.length / nMels)) is mathematically equivalent to expect(features.length).toBe(features.length) when the division is exact. Consider verifying against nMels * length instead.

🔧 Proposed fix

       const { features, length } = preprocessor.process(audio);
       expect(length).toBeGreaterThan(0);
-      expect(features.length).toBe(nMels * (features.length / nMels));
+      expect(features.length).toBe(nMels * length);
     }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const { features, length } = preprocessor.process(audio);
	expect(length).toBeGreaterThan(0);
	expect(features.length).toBe(nMels * (features.length / nMels));
	}
	const { features, length } = preprocessor.process(audio);
	expect(length).toBeGreaterThan(0);
	expect(features.length).toBe(nMels * length);
	}

🤖 Prompt for AI Agents

In `@tests/mel-extended.test.mjs` around lines 636 - 639, The test contains a
tautological assertion: replace the check of features length to validate it's
equal to nMels times the returned frame count; specifically, in the test that
destructures { features, length } from preprocessor.process(audio) and uses
nMels, change the assertion from expecting features.length to equal nMels *
(features.length / nMels) to expecting features.length toBe(nMels * length) so
it actually verifies features were produced for each mel bin per frame.

[ysdede]

Summary: Tests fail on this PR because computeRawMel does not support startFrame on base.

Issue:
[tests/mel-extended.test.mjs:84] startFrame expectation fails; computeRawMel currently ignores startFrame, so frames 0..startFrame-1 are not zero.

Suggestion: Rebase on #29 (or include the startFrame implementation) or update the tests to match the current API. Then rerun npm test.

Tests: npm test (failed: tests/mel-extended.test.mjs)

[ysdede]

Code Review: Extended Unit Tests

The additional tests are comprehensive and cover important edge cases for the recent optimizations.

Findings:

• Coverage: Good coverage of the startFrame parameter and incremental processing logic.
• Dependency Notice (P2): These tests reference computeRawMel(audio, startFrame), which is introduced in PR Bolt: Optimize incremental mel processing #40. Merging this PR before Bolt: Optimize incremental mel processing #40 will result in test failures. It is recommended to merge Bolt: Optimize incremental mel processing #40 first or combine them.
• Numerical Precision (P3): The accuracy check uses a threshold of 1e-5. While reasonable for many audio tasks, ensure this matches the target precision for NeMo parity.