Skip to content

fix(asr): correct first-chunk padding and alignment - #46

Open
dashidhy wants to merge 2 commits into
NVIDIA:mainfrom
dashidhy:fix/cache-aware-first-chunk
Open

dashidhy wants to merge 2 commits into
NVIDIA:mainfrom
dashidhy:fix/cache-aware-first-chunk

Conversation

@dashidhy

@dashidhy dashidhy commented Sep 14, 2026

Copy link
Copy Markdown

What changed

The first audio block has no previous block to overlap. The current code still adds front padding and applies the crop used for later blocks.

This patch gives the first block its own layout: for Nemotron at R13, 105 real feature frames, with no added prefix or overlap crop. It also handles R0, short input, end-of-stream and reset. The existing 9-frame history and all later-block processing stay unchanged. Model weights, decoding rules and endpointing policy are unchanged.

Tests

Tested on an M2 Pro, macOS 26.6.2, with ASR/diarization/tests enabled, Metal enabled and TTS/microphone capture disabled.

  • CTest: 7/7 passed.
  • New regression: fails on original main; passes on CPU and Metal at R0/R3/R6/R13.
  • The full 668-second original-volume input matches the validated diagnostic fix at R3 and R13.
  • JFK concurrency: CPU and Metal passed, 4 threads × 2 iterations.
ctest --test-dir build/metal-asr --output-on-failure
build/metal-asr/bin/test_cache_stream_geometry MODEL.gguf --gpu -1
build/metal-asr/bin/test_cache_stream_geometry MODEL.gguf --gpu 0
build/metal-asr/bin/test_rnnt_concurrency MODEL.gguf \
  test_files/asr/wav/test/jfk.wav --gpu 0 --threads 4 --iters 2 \
  --right-context 13 --language auto

Limits: CUDA/Vulkan were not tested. Another local audio sample showed Metal concurrency failures on both main and an earlier revision of this fix; their cause is not established or addressed here. The full pre-commit command could not download ShellCheck. All other hooks passed individually, and the exact pinned ShellCheck 0.10.0 passed separately on all eight shell scripts.

Provenance

Both commits are DCO-signed. Existing license notices are preserved; the contribution is Apache-2.0. AI assistance (Codex) was used. No application code, audio, transcripts, models or logs are included; the new test generates its own input.

Local validation inputs only:

Summary by CodeRabbit

  • Bug Fixes

    • Improved streaming speech recognition by using the correct context for the first audio chunk.
    • Preserved full initial recognition output while handling overlap correctly for subsequent chunks.
    • Improved consistency across chunk sizes, end-of-stream processing, and forced endpoint resets.
    • Reset streaming state reliably when starting a new recognition session.
  • Tests

    • Added coverage for cache-aware streaming geometry, output consistency, reset behavior, end-of-stream handling, and optional model execution.

Signed-off-by: Hongyuan Du <du-hongyuan@outlook.com>
Signed-off-by: Hongyuan Du <du-hongyuan@outlook.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 8fda9e6f-6a98-441c-81c2-5e5799e64f6c

📥 Commits

Reviewing files that changed from the base of the PR and between a5b6953 and 6886de5.

📒 Files selected for processing (7)
  • src/asr/encoder/cache_aware_encoder.h
  • src/asr/encoder/fastconformer.cpp
  • src/asr/encoder/fastconformer.h
  • src/asr/runner.cpp
  • src/asr/runner.h
  • tests/cpp/asr/CMakeLists.txt
  • tests/cpp/asr/test_cache_stream_geometry.cpp

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The cache-aware encoder and runner now use a shorter first mel chunk and distinct steady-state chunk geometry. The encoder preserves first-chunk output, while the runner manages overlap, flushing, resets, and validation.

Changes

Cache-aware stream geometry

Layer / File(s) Summary
Chunk geometry contracts
src/asr/encoder/cache_aware_encoder.h, src/asr/encoder/fastconformer.h, src/asr/runner.h
Documentation and EncoderConfig::cache_first_chunk_mel_frames() define separate first-chunk and subsequent-chunk geometry. CacheStreamRunner tracks first_chunk_ and declares next_chunk_mel_frames().
First-chunk and steady-state processing
src/asr/encoder/fastconformer.cpp, src/asr/runner.cpp
The runner selects dynamic chunk sizes, manages initial context and later overlap, and applies the same geometry during tail flushing and resets. The encoder drops leading frames only for non-first chunks.
Geometry and streaming validation
tests/cpp/asr/CMakeLists.txt, tests/cpp/asr/test_cache_stream_geometry.cpp
A CTest target validates geometry, encoder equivalence, packetization, end-of-stream handling, forced EOU behavior, and reset behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CacheStreamRunner
  participant EncoderConfig
  participant FastConformer
  CacheStreamRunner->>EncoderConfig: calculate first-chunk mel length
  CacheStreamRunner->>FastConformer: process selected mel chunk
  FastConformer->>FastConformer: retain first output or drop later overlap
  CacheStreamRunner->>CacheStreamRunner: flush and reset chunk state
Loading

Merge Risk: ⚪ Minimal · up to 6886d

The cache-stream geometry change includes coverage for first chunks, steady-state overlap, EOF, reset, and forced-EOU behavior. No concrete merge-blocking issue was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 6 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: correcting first-chunk padding and alignment for ASR streaming.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 6 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant