Fix(audio) : Remove realtime safety violations from audio callback path

[sarv-tech]

Summary

This PR removes two real-time safety issues from the audio callback path.

Changes

• Removed dynamic buffer resizing from process_audio().
• Added a safe fallback that clears the output buffer and returns when frame counts exceed the preallocated capacity.
• Replaced std::cerr usage in AudioCommandDispatcher::drain_commands() with an atomic error flag.
• Added check_and_clear_error() for non-blocking error reporting from non-audio threads.
• Updated affected unit and integration tests.

Why

Heap allocations and blocking I/O are not suitable for real-time audio processing because they can introduce latency spikes and audio dropouts.

These changes ensure the audio callback path remains deterministic and free from dynamic allocation and console I/O.

Testing

• Updated integration tests for over-capacity frame handling.
• Updated dispatcher tests for atomic error reporting.
• Existing tests continue to pass.

[coderabbitai]

Warning

Review limit reached

@sarv-tech, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 44 minutes and 39 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10a7c211-8b95-4604-b1ed-6ce566a9ca8d

📥 Commits

Reviewing files that changed from the base of the PR and between fe1c799 and 1444dc6.

📒 Files selected for processing (5)

• src/audio/engine/audio_command_dispatcher.cpp
• src/audio/engine/audio_command_dispatcher.h
• src/audio/engine/audio_engine_process.cpp
• tests/integration/test_audio_engine.cpp
• tests/unit/test_audio_command_dispatcher.cpp

📝 Walkthrough

Walkthrough

Audio command lookup failures now set and expose an atomic error flag instead of logging. Audio processing now zeroes the caller output and returns early when the requested frame count exceeds internal buffer capacity, with tests updated for both behaviors.

Changes

Audio real-time safety

Layer / File(s)	Summary
Dispatcher error flag src/audio/engine/audio_command_dispatcher.h, src/audio/engine/audio_command_dispatcher.cpp, tests/unit/test_audio_command_dispatcher.cpp	Lookup failures set an atomic error flag, a private helper reads and clears it, and the unit test checks the flag after an invalid lookup.
Oversized callback handling src/audio/engine/audio_engine_process.cpp, tests/integration/test_audio_engine.cpp	process_audio now zeroes the interleaved output buffer and returns early when frame_count exceeds capacity, and the integration test checks that buffers do not resize and output is silenced.

🎯 3 (Moderate) | ⏱️ ~20 minutes

I’m a rabbit with twitchy ears,
I hopped past logs and silent fears.
I zeroed buffers, neat and bright,
and set the error flag just right. 🐰
The audio trail now feels so light.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: removing realtime-safety violations from the audio callback path.
Linked Issues check	✅ Passed	The changes address the linked issue by removing callback-time resizing and console I/O, and by adding deferred error signaling.
Out of Scope Changes check	✅ Passed	The modified files and behavior stay within the audio real-time safety fix described in the linked issue.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

tests/integration/test_audio_engine.cpp (1)

535-543: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Derive the oversize case from the engine’s actual buffer capacity.

Line 535 and Line 536 hard-code the current default capacity. If that preallocation changes, this test can stop exercising the fallback path. Build large_frame_count from engine.test_process_buffer().size() + 1 instead.

Proposed change

-    // Default internal buffer capacity is 16384. Request a size larger than this.
-    const int large_frame_count = 17000;
+    const int large_frame_count =
+        static_cast<int>(engine.test_process_buffer().size()) + 1;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_audio_engine.cpp` around lines 535 - 543, The oversize
test case in the audio engine integration test is hard-coded to the current
default buffer size, which makes it fragile if the capacity changes. In the test
around engine.process_audio and engine.test_process_buffer(), derive
large_frame_count from the engine’s actual process buffer size plus one, then
use that value for the input/output vectors and the assertion so the fallback
path is always exercised regardless of future capacity changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/audio/engine/audio_command_dispatcher.cpp`:
- Around line 101-108: The dummy_effects fallback in
audio_command_dispatcher.cpp should not mark an error on successful lookup.
Update the node_id handling in the fallback path inside the audio command
dispatcher so that error_flag_ is only set when the lookup truly fails and
nullptr is returned, while the valid return dummy_effects[node_id] path remains
a normal success case.
- Around line 104-108: `AudioCommandDispatcher::check_and_clear_error()` is
never consumed in production, so the `error_flag_` set in
`audio_command_dispatcher.cpp` can stay latched forever. Update the main/audio
processing flow in `audio_engine_process.cpp` so the loop that calls
`drain_commands()` also invokes `dispatcher.check_and_clear_error()` afterward,
or route that call through a GUI/main-thread error handling path. Keep the fix
centered on `AudioCommandDispatcher`, `drain_commands()`, and
`check_and_clear_error()` so the flag is reliably cleared outside tests.

In `@tests/unit/test_audio_command_dispatcher.cpp`:
- Around line 93-95: The current test in test_audio_command_dispatcher.cpp does
not isolate the invalid node lookup path because valid node_id 0 commands may
already set dispatcher.error_flag_ via the dummy_effects fallback. Update the
test to first assert the flag stays clear after the valid-only pushes, then
trigger the node_id 99 out-of-bounds push and assert check_and_clear_error()
becomes true. Use AudioCommandDispatcher and check_and_clear_error() to keep the
intent explicit and regression-proof.

---

Nitpick comments:
In `@tests/integration/test_audio_engine.cpp`:
- Around line 535-543: The oversize test case in the audio engine integration
test is hard-coded to the current default buffer size, which makes it fragile if
the capacity changes. In the test around engine.process_audio and
engine.test_process_buffer(), derive large_frame_count from the engine’s actual
process buffer size plus one, then use that value for the input/output vectors
and the assertion so the fallback path is always exercised regardless of future
capacity changes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1206b43-9f11-48e3-a3b6-174603d3eca7

📥 Commits

Reviewing files that changed from the base of the PR and between 106d0e4 and fe1c799.

📒 Files selected for processing (5)

• src/audio/engine/audio_command_dispatcher.cpp
• src/audio/engine/audio_command_dispatcher.h
• src/audio/engine/audio_engine_process.cpp
• tests/integration/test_audio_engine.cpp
• tests/unit/test_audio_command_dispatcher.cpp

[sarv-tech]

Hi @sudip-mondal-2002 I've created a PR . Please review and approve