test: add audio graph stress and mutation coverage

[YASHcode-IIITV]

What does this PR do?

Adds additional stress and runtime mutation coverage for the audio graph system.

New test coverage includes:

• Repeated graph rebuild stress validation
• Repeated executor compile/process cycle testing
• Dynamic topology mutation and recompilation validation
• Output stability checks using finite-value assertions

These tests improve regression coverage for the DAG-based audio graph executor and runtime graph rebuild behavior.

---

Related Issue

Fixes #158

---

Type of Change

• 🐛 Bug fix
• ✨ New feature
• ♻️ Refactor / code cleanup
• 📝 Documentation
• ✅ Tests
• 🔧 Build / CI / Configuration

---

How Was This Tested?

• Platform(s) tested on: Windows 11
• Test command run: .\build\Release\amplitron-tests.exe

Manual test steps

1. Built the project using CMake with vcpkg dependencies
2. Ran the complete test suite
3. Verified all newly added stress/runtime graph tests pass successfully
4. Confirmed output buffers remain finite and stable during repeated graph recompilation and mutation scenarios

---

Checklist

• Code compiles and builds successfully on my platform
• All existing tests pass (./amplitron-tests)
• New tests added for new functionality (if applicable)
• Documentation updated (README, code comments) if applicable
• No blocking calls on the audio thread (no std::mutex::lock() on the hot path)
• Tested on: Windows 11

---

Screenshots / Demo

N/A — test-only changes

Summary by CodeRabbit

• Tests
  • Enhanced test coverage for audio engine operations including graph state management and parameter updates
  • Expanded audio graph executor robustness and topology validation testing

[coderabbitai]

Warning

Review limit reached

@YASHcode-IIITV, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 7 minutes and 22 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

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

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6f37384-cdbd-48b5-8f01-533f91ec94fb

📥 Commits

Reviewing files that changed from the base of the PR and between 99da42e and 81037b3.

📒 Files selected for processing (2)

• tests/test_audio_engine.cpp
• tests/test_audio_graph.cpp

📝 Walkthrough

Walkthrough

This PR adds 546 lines of new unit tests across test_audio_engine.cpp and test_audio_graph.cpp to exercise AudioGraph rebuild cycles, topology mutations, state management, and validation rules under stress conditions, plus serialization roundtrips for engine state preservation.

Changes

Audio System Stability and Stress Testing

Layer / File(s)	Summary
AudioGraph rebuild, recompile, and mutation stress tests tests/test_audio_graph.cpp	Three stress tests repeatedly construct, recompile, and dynamically mutate graph topologies (splitter→effect→merge patterns and node insertion), asserting output samples remain finite and bounded.
Executor behavior edge cases and buffer handling tests/test_audio_graph.cpp	Tests verify executor process() behavior when compile() is not explicitly called, and passthrough handling for input blocks larger than the prepared buffer size.
Graph topology validation and constraint enforcement tests/test_audio_graph.cpp	Tests enforce graph invariants: duplicate links return the existing id, input pins reject multiple incoming links, output pins reject multiple outgoing links, link removal succeeds or fails appropriately, and invalid node/pin queries return expected sentinel values.
Implicit input selection, sink detection, and unreachable node exclusion tests/test_audio_graph.cpp	Tests cover fallback input selection when no explicit input is set, implicit sink detection without explicit output marking, exclusion of unreachable nodes from processing, and combined fallback paths when neither input nor output nodes are explicitly designated.
AudioEngine graph commit stability and operations tests/test_audio_engine.cpp	Tests verify repeated commit_graph_changes() calls on populated and empty graphs, commit behavior following node removal, and basic success assertions for the commit operation.
AudioEngine buffer size and sample rate state updates tests/test_audio_engine.cpp	Tests update buffer size with out-of-range values (negative and extremely large) and sample rate across multiple valid rates and repeated iterations, ensuring state mutations do not crash.
AudioEngine serialization and deserialization roundtrip tests/test_audio_engine.cpp	Test serializes an engine, deserializes into a new AudioEngine, re-serializes, and asserts both payloads contain "input_gain" to verify state preservation across the roundtrip.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• sudip-mondal-2002/Amplitron#178: Adds AudioGraph preset/JSON roundtrip coverage that depends on graph node serialization state, aligning directly with this PR's engine serialization tests and graph state validation.

Suggested labels

type:testing, Audio Processing, level:advanced

Suggested reviewers

• sudip-mondal-2002

Poem

🐰 Stress-test the graphs, rebuild with care,
Mutation flows through the audio air,
Validation rules catch the edge-case fray,
Serialization roundtrips save the day! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	PR adds test coverage but falls significantly short of linked issue #158 requirements. Only partial implementation: graph stress tests present but lack concurrent thread-safety tests, ThreadSanitizer integration, and several critical concurrent mutation scenarios.	Implement the 5 required concurrent test cases (add-node, remove-node during playback, rapid preset switching, buffer size guard, disconnected node silence) and integrate ThreadSanitizer in CI as specified in issue #158.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main addition of stress and mutation test coverage for the audio graph, aligning with the changeset's focus on new test cases.
Out of Scope Changes check	✅ Passed	All changes are in-scope test additions. However, the implementation falls short of the linked issue's specific objectives and acceptance criteria.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[github-actions]

Code Coverage Report 📊

Line Coverage: 79.6%

✅ Coverage meets threshold: 79.6% >= 60%

Full Coverage Summary

Summary coverage rate:
  lines......: 79.6% (3345 of 4203 lines)
  functions..: 89.9% (474 of 527 functions)
  branches...: no data found

[github-actions]

PR Preview Removed

The GitHub Pages preview for this PR has been removed because the PR was closed.

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/test_audio_graph.cpp (1)

349-894: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Add ThreadSanitizer + concurrent graph-mutation coverage (issue #158)

• tests/test_audio_graph.cpp (lines 349-894) only rebuilds/compiles/processes graphs sequentially and contains no threading primitives, so it won’t exercise the hot-audio swap path (AudioEngine::commit_graph_changes() handing off main_executor_ to audio_shadow_executor_) under concurrent access.
• There’s no ThreadSanitizer enablement in the repo build/CI: no TSAN/sanitize_thread/AMPLITRON_TSAN options are present in CMakeLists.txt or scripts (only -pthread linkage), so the new tests can’t be run under TSan as required by issue #158.

🤖 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/test_audio_graph.cpp` around lines 349 - 894, Tests currently only run
sequentially and never exercise concurrent graph handoff
(AudioEngine::commit_graph_changes and the main_executor_ ->
audio_shadow_executor_ swap) nor is ThreadSanitizer enabled in the build; add a
new stress test that performs concurrent graph mutations and executor
compile/process on separate threads to exercise commit_graph_changes (reference
AudioEngine::commit_graph_changes, main_executor_, audio_shadow_executor_,
AudioGraphExecutor::compile, AudioGraphExecutor::process, and
AudioGraph::rebuild_topology) and update the CMake configuration to enable
TSAN/sanitize=thread (e.g. add an AMPLITRON_TSAN or sanitize_thread option that
injects -fsanitize=thread and appropriate linker flags) so the test can run
under ThreadSanitizer in CI.

🧹 Nitpick comments (1)

tests/test_audio_graph.cpp (1)

834-865: 💤 Low value

Consider verifying exact output value in fallback path test.

The test correctly exercises fallback input and implicit sink detection when no explicit input/output nodes are designated, but only asserts finite output. Consider also verifying that output[0] == 0.5f to confirm the passthrough chain A→B processes correctly.

🔍 Optional assertion to strengthen verification

   executor.process(input.data(), output.data(), 64);

   for (float sample : output) {
     ASSERT_TRUE(std::isfinite(sample));
   }
+
+  ASSERT_TRUE(output[0] == 0.5f);

🤖 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/test_audio_graph.cpp` around lines 834 - 865, The test currently only
asserts output samples are finite; strengthen it by asserting the expected
passthrough value from the fallback input→A→B→sink path: after
executor.process(input.data(), output.data(), 64) add a deterministic equality
check (for example ASSERT_EQ or ASSERT_FLOAT_EQ) that output[0] == 0.5f (or
ASSERT_NEAR with a tiny epsilon if floating-point noise is possible) to verify
AudioGraph/AudioGraphExecutor correctly routes and preserves the sample through
nodes A and B.

🤖 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 `@tests/test_audio_engine.cpp`:
- Around line 388-404: The tests audio_engine_set_buffer_size_clamps_values and
audio_engine_set_sample_rate_updates_state currently assert true
unconditionally; replace those no-op assertions with real checks that verify
behavior: after calling AudioEngine::set_buffer_size(-1) and
set_buffer_size(999999) assert the engine reports clamped values (use the actual
accessor, e.g., AudioEngine::get_buffer_size() or buffer_size()) equal to the
defined minimum and maximum buffer sizes; and in
audio_engine_set_sample_rate_updates_state assert that
AudioEngine::get_sample_rate() (or sample_rate()) matches the last set value
(48000 then 96000) or the nearest supported rate if the implementation
clamps/rounds — update the tests to call the correct accessor names present in
the class and compare against the expected constants or values.
- Around line 443-456: The test audio_engine_serialize_deserialize_roundtrip
currently only checks that the "input_gain" key exists; update it to verify
value preservation by setting a non-default input gain on AudioEngine (e.g.,
call engine.setInputGain(...) or modify the engine's input_gain field) before
calling engine.serialize(), then after loaded.deserialize(serialized) assert
that the loaded engine's input gain equals the original value (via
loaded.getInputGain() or by reading the "input_gain" value from reserialized
JSON) so the roundtrip verifies the actual value, not just key presence.

In `@tests/test_audio_graph.cpp`:
- Around line 527-573: The test
audio_graph_repeated_executor_recompile_processing only performs sequential
compile/process loops and must be changed to exercise concurrent graph mutation
during audio callbacks: spawn a background thread that repeatedly builds a new
AudioGraph (using AudioGraph::add_node, set_node_as_input, set_node_as_output,
add_link, rebuild_topology) and calls executor.compile(...) or an atomic swap
API while the main thread continuously calls AudioGraphExecutor::process(...)
(after AudioGraphExecutor::prepare). Ensure proper synchronization to stop the
background thread at test end and verify outputs remain finite and bounded as
before; the goal is to validate atomic graph swaps while processing rather than
sequential-only recompiles.
- Around line 349-402: The test currently runs sequential graph rebuilds but
must exercise concurrent mutations per issue `#158`: modify
audio_graph_rapid_graph_rebuild_stress to start continuous processing on a
background thread using AudioGraphExecutor::process (or a wrapper that
repeatedly calls executor.process for ~1000 callbacks), then from the main
thread perform concurrent graph mutations—use graph.add_node, graph.add_link,
graph.set_node_as_input/output and call
graph.rebuild_topology()/commit_graph_changes() mid-stream to trigger the
lock-free commit path; ensure the background thread reads from the same
executor/graph instance while the main thread mutates, loop ~1000 process
callbacks, validate output samples are finite after each callback, and join the
worker thread before test exit to avoid races.
- Around line 575-656: The test currently adds a node to the original graph but
instead builds and uses a new rebuilt_graph; change it to mutate graph in-place:
after calling graph.add_node("InsertedEffect", NodeRoutingType::StandardEffect)
modify links on graph (use graph.get_nodes() to find pins and
graph.add_link/remove_link as needed), call graph.rebuild_topology(), then call
executor.compile(graph) and executor.process(...) to validate the mutated
topology outputs (keep the same finite and range assertions). Ensure you
reference the existing variables graph, executor, and the methods
graph.add_node, graph.add_link, graph.rebuild_topology, and executor.compile so
the test actually exercises dynamic topology mutation.
- Around line 670-682: The test audio_graph_executor_oversized_block_passthrough
only verifies that the first max_block_size_ samples are copied and does not
assert any rejection or error behavior for oversized inputs; update the test
and/or implementation so oversized blocks are properly signaled. Modify the test
to either (a) expect and assert a defined error/boolean/status from
AudioGraphExecutor::process (change its signature if needed) when num_samples >
max_block_size_, or (b) explicitly assert that the remaining samples in output
beyond max_block_size_ are unchanged/zeroed or that a log/flag was set; locate
AudioGraphExecutor::process and the test
audio_graph_executor_oversized_block_passthrough to implement the chosen
approach and add assertions for the post-max_block_size_ region or for the new
error/status return to ensure oversized input is caught.

---

Outside diff comments:
In `@tests/test_audio_graph.cpp`:
- Around line 349-894: Tests currently only run sequentially and never exercise
concurrent graph handoff (AudioEngine::commit_graph_changes and the
main_executor_ -> audio_shadow_executor_ swap) nor is ThreadSanitizer enabled in
the build; add a new stress test that performs concurrent graph mutations and
executor compile/process on separate threads to exercise commit_graph_changes
(reference AudioEngine::commit_graph_changes, main_executor_,
audio_shadow_executor_, AudioGraphExecutor::compile,
AudioGraphExecutor::process, and AudioGraph::rebuild_topology) and update the
CMake configuration to enable TSAN/sanitize=thread (e.g. add an AMPLITRON_TSAN
or sanitize_thread option that injects -fsanitize=thread and appropriate linker
flags) so the test can run under ThreadSanitizer in CI.

---

Nitpick comments:
In `@tests/test_audio_graph.cpp`:
- Around line 834-865: The test currently only asserts output samples are
finite; strengthen it by asserting the expected passthrough value from the
fallback input→A→B→sink path: after executor.process(input.data(),
output.data(), 64) add a deterministic equality check (for example ASSERT_EQ or
ASSERT_FLOAT_EQ) that output[0] == 0.5f (or ASSERT_NEAR with a tiny epsilon if
floating-point noise is possible) to verify AudioGraph/AudioGraphExecutor
correctly routes and preserves the sample through nodes A and B.

🪄 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: 6a54054c-5ba3-429d-aa11-fc0f3346b00b

📥 Commits

Reviewing files that changed from the base of the PR and between cd648a0 and 99da42e.

📒 Files selected for processing (2)

• tests/test_audio_engine.cpp
• tests/test_audio_graph.cpp

[sudip-mondal-2002]

LGTM