Focus code analysis#40
Merged
davidirvine merged 1 commit intodevelopfrom Jul 23, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR focuses the CodeQL security analysis to only analyze source code in the src/ directory rather than the entire codebase. The changes optimize CI/CD performance by limiting analysis scope and excluding test code, documentation, and build artifacts.
- Adds path filters to trigger CodeQL only when relevant files change
- Creates a custom CodeQL configuration to limit analysis scope to source code
- Updates build process comments to reflect the focused analysis approach
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/codeql.yml |
Adds path filters for workflow triggers and references custom config file |
.github/codeql/codeql-config.yml |
New configuration file that restricts analysis to src/ directory and excludes tests/docs |
| cmake -DCMAKE_BUILD_TYPE=Release .. | ||
| make | ||
| # Build only the source files needed for analysis | ||
| make -j2 |
There was a problem hiding this comment.
The hardcoded parallel job count (-j2) may not be optimal for all CI environments. Consider using make -j$(nproc) to utilize all available CPU cores or make this configurable.
Suggested change
| make -j2 | |
| make -j$(nproc) |
| - "build/**" | ||
| - "**/*.md" | ||
| - "**/*.txt" | ||
| - "**/CMakeLists.txt" |
There was a problem hiding this comment.
[nitpick] Excluding CMakeLists.txt files from analysis may miss security issues in build configurations. Consider whether build script analysis provides value for your security posture.
Suggested change
| - "**/CMakeLists.txt" | |
| # 17: - "**/CMakeLists.txt" |
davidirvine
added a commit
that referenced
this pull request
Jul 25, 2025
* feat: replace dynamic allocation with fixed-size arrays - Replace std::vector with std::array for oscillators storage - Add MAX_OSCILLATORS constant to prevent unbounded allocation - Eliminate dynamic memory allocation in audio processing path - Add oscillator_count member to track active oscillators - Update all oscillator iteration to use index-based loops This improves real-time audio performance by avoiding heap allocation during audio processing, which is critical for maintaining low latency. * feat: add comprehensive parameter validation - Add validation for sample rate, frequencies, and oscillator count - Throw std::invalid_argument with descriptive messages for invalid parameters - Validate non-negative frequencies and positive sample rates - Check oscillator count bounds against MAX_OSCILLATORS - Add validation for animation multiplier - Include <stdexcept> for exception handling This prevents undefined behavior from invalid parameters and provides clear error messages to help developers identify configuration issues. * feat: improve const correctness and add noexcept specifications - Add noexcept to getter methods and simple setters that cannot throw - Mark const methods with noexcept where appropriate - Update Range class methods with noexcept specifications - Improve exception safety guarantees for performance-critical operations This helps the compiler optimize better and provides clearer contract specifications for API consumers about which operations can throw. * feat: centralize magic numbers into constants namespace - Create deepnote::constants namespace for configuration values - Define DEFAULT_LFO_AMPLITUDE, DEFAULT_DETUNE_HZ, TARGET_FREQUENCY_TOLERANCE - Replace hard-coded magic numbers with named constants - Update init_voice function to use DEFAULT_DETUNE_HZ constant - Update target frequency tolerance in process_voice function This improves code maintainability by centralizing configuration values and making their purpose explicit through naming. * refactor: break down complex process_voice function into smaller functions - Extract calculate_shaped_frequency for LFO processing and scaling - Extract update_voice_state for state transition logic - Extract constrain_frequency for frequency bounds checking - Simplify main process_voice function to focus on orchestration - Improve code readability and testability by separation of concerns - Use anonymous namespace for helper functions to limit scope This makes the code more maintainable and easier to understand by breaking complex logic into focused, single-responsibility functions. * docs: add comprehensive documentation and comments - Add detailed class documentation for DeepnoteVoice with usage instructions - Document BezierUnitShaper with mathematical formula and purpose - Add comprehensive function documentation with parameter descriptions - Document the detuning algorithm with clear explanation - Add examples and usage patterns in docstrings - Explain the state machine and animation system This improves code maintainability and helps developers understand the complex audio processing algorithms and their intended usage. * feat: enhance build system and add development tooling - Add comprehensive compiler warning flags (-Wall, -Wextra, -Wpedantic, -Werror) - Enable AddressSanitizer and UndefinedBehaviorSanitizer in debug builds - Add optimized release builds with -O3 - Create .clang-format configuration for consistent code formatting - Enhance .gitignore with comprehensive build artifact exclusions - Add static analysis script for clang-format, clang-tidy, and cppcheck - Improve development workflow with automated code quality checks This establishes better development practices and helps catch bugs early through static analysis and runtime sanitizers. * fix: tone down aggressive compiler flags to work with external dependencies - Remove -Werror and -Wpedantic to avoid failures from external code - Add more warning exclusions for common issues in external dependencies - Remove sanitizers from default debug build to reduce development friction - Keep reasonable warnings (-Wall -Wextra) for our own code quality This allows the project to build successfully while still maintaining good code quality standards for the main codebase. * Fix voice state machine bug in refactored helper functions The issue was in the update_voice_state helper function which was re-reading the voice state instead of using the current state passed from the main processing loop. This caused the voice to get stuck in PENDING_TRANSIT_TO_TARGET state instead of properly transitioning through IN_TRANSIT_TO_TARGET to AT_TARGET. Changes: - Modified update_voice_state to accept current_state parameter - Fixed enum references to use DeepnoteVoice::STATE instead of voice.STATE - All tests now pass (10 passed, 0 failed) * feat: add CI/CD pipeline and documentation improvements - Implement CI/CD workflow in .github/workflows/ci.yml for automated testing and deployment - Enhance .gitignore to exclude generated documentation files - Create detailed improvement suggestions in IMPROVEMENT_SUGGESTIONS.md - Add Doxygen configuration in docs/doxygen/Doxyfile for API documentation - Develop mainpage.md for Doxygen documentation overview - Provide a basic usage example in docs/examples/basic_usage.cpp - Introduce performance guide in docs/performance_guide.md for optimization strategies * Refactor FrequencyTable and OscillatorFrequency - Moved FrequencyTableIndex and VoiceIndex definitions to the nt namespace in frequencytable.hpp. - Updated FrequencyTable to use a 2D array of frequency functions, improving encapsulation and readability. - Simplified the get method in FrequencyTable to handle out-of-bounds indices with wrapping. - Cleaned up oscfrequency.hpp by removing unnecessary namespace brackets. - Adjusted test cases in freqtable.cpp for improved formatting and readability. - Made minor formatting adjustments across various test files for consistency. - Ensured proper inclusion of headers in voice.cpp and main.cpp. * Fix CI/CD pipeline and code formatting - Updated GitHub Actions workflow to support feature branches - Fixed Doxygen configuration and documentation paths - Reformatted all source files to meet clang-format requirements - Improved CI error handling and cross-platform compatibility - Updated .gitignore to exclude generated documentation * fix: include <memory> in voice.cpp for unique_ptr usage * refactor: improve constructor formatting in FrequencyTable and update compiler warnings in CMakeLists * fix: correct constructor formatting in FrequencyTable and include missing <string> header in deepnotevoice.hpp * fix: improve constructor formatting in FrequencyTable for better readability * ugh * fix: adjust constructor formatting in FrequencyTable for consistency * ugh ugh * fix: update update_voice_state function signature and improve cppcheck integration in CI * fix: align variable declarations for improved readability in process_voice function * fix: update CI conditions to include fixing-cicd-pipeline branch for coverage, documentation, and benchmarks * testing * fix: update CI permissions and improve Codecov integration * fix: enhance CI workflow with secrets and permissions checks * fix: update CI configuration to include fixing-* branch and enhance secrets handling * fix: remove CODECOV_TOKEN environment variable from debug job * fix: update CI configuration to remove debug job and restrict branch triggers * fix: restore conditions for coverage and documentation jobs, update GitHub Pages deployment settings * spelling fix and added link to doxygen docs * Native github code coverage (#37) * Use native github code coverage Replace Codecov with: lcov-reporter-action for PR comments Custom badge generation for README coverage badges Artifact upload to store the badge * codeql workflow * feat: add CodeQL configuration and update workflow for targeted analysis (#40) * fix: remove CMakeLists.txt from paths-ignore in CodeQL config (#41) * Performance tests (#42) * feat: Add comprehensive performance and real-time validation tests - Implement real-time processing validation * Single voice processing speed verification (sub-100ms for 1s audio) * Multiple voice scaling tests (1-16 voices with CPU usage monitoring) * Real-time constraint validation for different voice counts - Add memory allocation stability tests * Long-duration processing without dynamic allocation * Consistent memory usage across multiple transitions * Amplitude and frequency bounds verification during extended processing - Implement stress testing * Rapid parameter changes with fast animation speeds * Extreme oscillator count testing (up to 16 oscillators) * High-complexity scenarios with detuning and rapid transitions - Add performance regression detection * Baseline performance measurement with detailed timing statistics * Per-sample processing time analysis (average, median, 95th percentile, max) * Performance consistency across different voice states - Fix duplicate symbol issue with inline specifier for init_voice function - Update CMakeLists.txt to include performance_tests.cpp - All tests pass with 606,406 assertions validating performance characteristics - Tests maintain C++14 compatibility and realistic performance expectations * re-formatted * Property tests (#44) * feat: Add comprehensive performance and real-time validation tests - Implement real-time processing validation * Single voice processing speed verification (sub-100ms for 1s audio) * Multiple voice scaling tests (1-16 voices with CPU usage monitoring) * Real-time constraint validation for different voice counts - Add memory allocation stability tests * Long-duration processing without dynamic allocation * Consistent memory usage across multiple transitions * Amplitude and frequency bounds verification during extended processing - Implement stress testing * Rapid parameter changes with fast animation speeds * Extreme oscillator count testing (up to 16 oscillators) * High-complexity scenarios with detuning and rapid transitions - Add performance regression detection * Baseline performance measurement with detailed timing statistics * Per-sample processing time analysis (average, median, 95th percentile, max) * Performance consistency across different voice states - Fix duplicate symbol issue with inline specifier for init_voice function - Update CMakeLists.txt to include performance_tests.cpp - All tests pass with 606,406 assertions validating performance characteristics - Tests maintain C++14 compatibility and realistic performance expectations * feat: Add comprehensive property-based and edge case validation tests - Implement frequency transition properties validation * Random parameter testing with 50 test cases across musical frequency range * Monotonicity property verification for ascending frequency transitions * Adaptive timeouts based on animation speed for reliable completion - Add extreme parameter value testing * Very low frequencies (20-40Hz sub-bass range) stability validation * Very high frequencies (8-18kHz treble range) without aliasing issues * Extreme frequency ratio testing (32x increases/decreases across 5+ octaves) * Error tolerance validation within 10% for extreme transitions - Implement Bezier curve parameter edge cases * Boundary value testing for control points (0.0-1.0 range) * Extreme control point combinations (linear, reverse, near-extremes) * Animation multiplier testing (0.5x to 10x speed variations) * Graceful handling of edge curve configurations - Add detuning edge case validation * Extreme detune values (0Hz to ±100Hz) stability testing * Negative detuning support verification * Detuning behavior during frequency transitions with amplitude variation * Beat pattern validation with reasonable amplitude bounds - Implement sample rate variation testing * Multi-rate support (8kHz to 192kHz) stability validation * Sample rate and frequency relationship verification * Proportional timing behavior across different sample rates - Update CMakeLists.txt to include property_edge_tests.cpp - All tests pass with 1,958,075 assertions validating comprehensive edge coverage - Property-based testing with random parameter generation for robust validation - Realistic bounds and tolerances for practical audio applications * re-formatted * feat: add state machine lifecycle tests for deepnote voice (#48) * feat: add state machine lifecycle tests for deepnote voice * fix: improve static analysis script and enhance performance tests with additional metrics * feat: add bezier animation tests to validate curve properties and ani… (#49) * feat: add bezier animation tests to validate curve properties and animation accuracy * Update test/bezier_animation_tests.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: add error robustness tests to validate system behavior under ed… (#50) * feat: add error robustness tests to validate system behavior under edge cases * Fix robustness tests and improve performance guidance - Fixed detuning boundary tests by using more reasonable extreme values (±200Hz instead of ±500Hz) - Reduced oscillator count for extreme detuning tests to prevent excessive amplitudes - Separated normal and extreme detuning tests for better reliability - Updated performance guide with detuning amplitude considerations - Added amplitude management section to performance guide - All 34 test cases now pass with 2,197,690 total assertions Changes ensure robustness tests are reliable in CI environments while still validating system behavior under stress conditions. * feat: enhance boundary condition tests for detuning values and improve robustness checks * Fix build issues in test files - Add missing <chrono> header include to error_robustness_tests.cpp - Suppress unused variable warnings in performance_tests.cpp with (void)output statements - Apply clang-format styling corrections - Fixes compilation errors that were preventing CI builds from succeeding * fix: enable coverage job for pull requests targeting develop - Update coverage job condition to run on develop branch pushes AND pull requests targeting develop - This ensures coverage reports are generated during PR review process - Coverage report comments will now appear on PRs as intended - Badge generation remains restricted to develop branch only * fix: update deprecated actions/upload-artifact from v3 to v4 - Replace actions/upload-artifact@v3 with @v4 to resolve CI deprecation warning - Fixes: 'This request has been automatically failed because it uses a deprecated version of actions/upload-artifact: v3' - Coverage badge upload will now use the supported artifact action version * fix: update lcov-reporter-action and add pull-requests permission - Update romeovs/lcov-reporter-action from v0.3.1 to v0.4.0 to resolve 403 permission errors - Add pull-requests: write permission to workflow to allow coverage comments on PRs - Fixes: 'Resource not accessible by integration' error in coverage reporting - Ensures coverage reports can be posted as PR comments successfully --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.