feat: SQLite optimizations, dashboard enhancements, and MKV attachment fixes - #1
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces comprehensive SQLite optimizations, enhanced telemetry and dashboard systems, improved webhook processing, development tooling, and critical bug fixes to modernize Reencodarr's architecture and reliability.
Key Changes:
- Database Migration: Complete migration from PostgreSQL to SQLite with advanced concurrency features via WAL mode
- Performance Monitoring: Enhanced analyzer performance tracking with throughput metrics and dashboard integration
- Code Quality: Automated pre-commit hooks with Credo strict checks and formatting validation
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
config/config.exs |
Centralized SQLite performance optimizations with WAL mode, large cache, and memory mapping |
config/dev.exs, config/test.exs, config/runtime.exs |
Environment-specific database configuration updates for SQLite |
lib/reencodarr/analyzer/broadway/performance_monitor.ex |
Enhanced performance monitoring with throughput tracking and telemetry emission |
lib/reencodarr/telemetry_reporter.ex |
Added analyzer throughput tracking with queue-based progress calculation |
lib/reencodarr_web/components/dashboard_components.ex |
Enhanced dashboard components with analyzer-specific progress display |
lib/reencodarr_web/controllers/radarr_webhook_controller.ex |
Streamlined webhook processing for more efficient video record creation |
lib/reencodarr/media.ex |
Code quality improvements with better function organization and error handling |
.githooks/pre-commit, lib/mix/tasks/setup_precommit.ex |
Automated code quality enforcement via git hooks |
.github/copilot-instructions.md |
Updated documentation reflecting SQLite architecture and development patterns |
mjc
force-pushed
the
feature/sqlite-optimizations-and-fixes
branch
from
September 10, 2025 18:12
4afa36d to
76df72f
Compare
- Add gnupg, pinentry, and pinentry-curses to development shell - Configure GPG_TTY and pinentry program in shellHook - Set git to use nix-provided GPG binary for consistent behavior
- Add proper error handling for invalid video IDs in VMAF upserts - Refactor VMAF handling with helper functions for better organization - Add skipping logic for already encoded videos - Improve logging and PubSub notifications for CRF search events
- Move common SQLite settings to config.exs - Enable WAL mode and concurrent access settings - Configure binary storage for arrays and maps - Set base pragma values for all environments feat: optimize development database settings - Increase connection pool size to 20 for better concurrency - Configure 100MB SQLite cache for development - Add 256MB memory mapping for better performance - Set 60s busy timeout for long operations feat: streamline test database configuration - Remove duplicate settings now in config.exs - Configure sandbox-specific settings - Optimize cache size for test environment - Set appropriate timeouts for tests refactor: simplify production database configuration - Remove unused database_url configuration - Set default database path with override via DATABASE_PATH - Clean up unused IPv6 configuration - Keep production-optimized SQLite settings chore: improve SQLite config readability with number formatting feat: consolidate SQLite optimizations in base config - Move all SQLite performance tuning to config.exs - Add comprehensive concurrency improvements: - WAL mode for better reader/writer concurrency - 256MB cache size for performance - 512MB memory mapping for I/O performance - 2-minute busy timeout for concurrent operations - Apply optimizations across all environments refactor: remove duplicated SQLite pragma configs from environments - Remove pragma overrides from dev.exs, test.exs, and runtime.exs - Keep only environment-specific database settings - Ensure base config optimizations aren't overridden - Maintain single source of truth for SQLite performance tuning
- Add performance monitoring with adjustable batch sizes - Implement adaptive rate limiting based on throughput - Add mediainfo batch processing with configurable batch sizes - Improve error handling and logging for batch operations feat: improve dashboard state management and progress tracking - Add throughput and performance metrics to analyzer progress - Enhance state management with better status tracking - Add new fields for monitoring rate limits and batch sizes - Improve progress calculation and normalization feat: enhance progress normalization and presentation - Add throughput and performance metrics to progress normalization - Improve presenter logging for better debugging - Add rate limit and batch size to progress data structure - Enhance data transformation for UI presentation feat: update dashboard UI with performance metrics - Add analyzer-specific progress component with performance stats - Update LiveView to handle new performance metrics - Improve telemetry event handling and logging - Switch to TelemetryReporter for reliable state management feat: enhance telemetry system with performance monitoring - Add performance metrics to telemetry events - Improve throughput monitoring and reporting - Add analyzer-specific telemetry handling - Enhance state updates based on performance metrics
- Remove premature mediainfo processing from webhook handler - Set videos directly to needs_analysis state for proper pipeline flow - Clean up existing VMAFs when re-analyzing videos - Improve error handling and logging
feat: add pre-commit hook with credo and format checks - Create pre-commit hook script with strict code quality checks - Add mix setup_precommit task for easy hook configuration - Check formatting and run credo in strict mode - Safely handle unstaged changes during checks
- Add MKV attachment cleaning to CRF search and encoding operations - Use Helper.clean_mkv_attachments/1 to remove problematic attached images - Prevents FFmpeg exit code 218 errors from stream mapping conflicts - Maintains original behavior when no image attachments are present - Fixes encoding failures with files containing cover.jpg attachments
- Always run complete test suite instead of individual tests - Remove interactive commands (iex, phx.server) from development workflow - Emphasize manual sandbox mode requires full suite execution
- Remove redundant create_video function from Media module - Fix upsert_video to properly upsert videos by path using on_conflict - Remove max_audio_channels and atmos from required video fields - Standardize on upsert_video for all video creation operations Fix sync video creation to include proper audio metadata - Extract video parameters including max_audio_channels and atmos from mediainfo - Merge extracted parameters with basic video attributes in process_single_video_file - Ensure videos created via sync have complete metadata for Rules.audio() function - Convert atom keys to string keys for consistency with upsert_video expectations
- Add defensive nil check for TelemetryReporter in get_initial_state - Fix presenter function arity by adding explicit present/2 function - Prevent crashes when TelemetryReporter process is not started (test environment) - Ensure consistent fallback to initial dashboard state when needed
- Add video_fixture_with_result() function returning {:ok, video} tuples
- Fix video_fixture() to use Media.upsert_video instead of create_video
- Update DataCase documentation to reference upsert_video consistently
- Handle fixture creation errors gracefully with descriptive error messages
- Standardize fixture patterns for tests expecting different return types
Replace Media.create_video with fixtures across test suite
- Convert all test files to use Fixtures.video_fixture() instead of Media.create_video()
- Use video_fixture_with_result() for tests expecting {:ok, video} patterns
- Update pattern matching tests, savings tests, property tests, and integration tests
- Remove direct Media.create_video calls from all test files
- Standardize test data creation patterns throughout the codebase
- Added missing max_audio_channels and atmos fields to @optional list in Video schema - Fixes audio argument generation where Rules.audio/1 was receiving nil values - Resolves 8+ test failures related to audio codec arguments in Rules.build_args - Audio tests now properly generate ['--acodec', 'libopus', '--enc', 'b:a=256k'] args
- Modified process_single_video_file/2 to preserve file-derived metadata when file size unchanged - Only updates API-sourced fields (service_id, service_type, content_year, dateAdded) for unchanged files - Respects explicit bitrate=0 as re-analysis signal even when file size unchanged - Prevents unnecessary re-analysis of codecs, state, duration when file content hasn't changed - Fixes sync bitrate preservation tests: preserves analyzed bitrates during API sync operations - Refactored with helper functions to reduce nesting depth and improve code readability - Resolves final 2 test failures, achieving complete test suite success (493 tests, 0 failures)
- Changed test pattern from {:ok, _video} = to _video = for video_fixture call
- video_fixture returns video struct directly, not {:ok, video} tuple
- Fixes Broadway pipeline error resilience test for missing files
- Resolves MatchError: no match of right hand side value pattern issue
- Updated CRF search GenServer and integration test fixtures to use state: :analyzed - CrfSearch.crf_search/2 requires videos to be in :analyzed state to proceed - Videos in :needs_analysis state are rejected with :error return value - Fixes 'Assertion with == failed: left: :error, right: :ok' test failures - Ensures CRF search tests properly simulate analyzed video workflow - Resolves integration test failures in GenServer lifecycle and public API tests
mjc
force-pushed
the
feature/sqlite-optimizations-and-fixes
branch
from
September 10, 2025 18:56
552e1a4 to
7d5efbd
Compare
- Restore max_audio_channels and atmos as required fields - Fixes breaking schema changes identified by GitHub Copilot review - Ensures test fixture compatibility with existing validation
- Fix convenience functions to consistently return tuples
- Update create_test_video, create_opus_video, create_hdr_video, create_4k_video
- Ensures all fixtures follow {:ok, struct} pattern for destructuring
- Resolves KeyError issues when tests access .id on tuples vs structs
- Fix codec_optimization_test.exs: Remove direct imports, use DataCase alias, add tuple destructuring (4/4 tests passing)
- Fix media_test.exs: Fix factory pattern with proper tuple destructuring
- Fix rules_integration_test.exs: Fix setup function tuple destructuring for Broadway encoder test
- All use pattern: {:ok, video} = Fixtures.video_fixture(...) instead of direct assignment
- Resolves KeyError when accessing .id on tuples
- Fix savings_calculation_test.exs: Apply tuple destructuring to video fixture (5/5 tests passing)
- Fix pattern_matching_test.exs: Fix setup function for large file size warnings test
- Fix genserver_test.exs: Fix setup function tuple destructuring (7/7 tests passing)
- Consistent pattern: {:ok, video} = Fixtures.video_fixture(...)
- Fix savings_integration_test.exs: Fix all video fixture calls with tuple destructuring (3/3 tests passing)
- Fix sync_integration_test.exs: Fix sonarr_video and radarr_video fixture calls
- Fix video_queries_test.exs: Standardize excluded video fixtures for consistency
- Integration tests now properly handle {:ok, video} pattern from fixtures
- Fix all remaining AB-AV1 module tests with consistent tuple destructuring
- Fix analyzer_test.exs with proper fixture handling
- Fix encoder tests for exception handling and preset 6 encoding
- Fix failure tracking and reporting tests with tuple pattern
- Consistent {:ok, video} = Fixtures.video_fixture(...) pattern applied
- Fix integration tests including failure_tracking_integration_test.exs and preset_6_workflow_test.exs
- Fix media tests including exclude_patterns_test.exs with proper tuple handling
- Fix media_property_test.exs, media_savings_sort_test.exs, savings_core_test.exs
- Fix sync_bitrate_preservation_test.exs and video_processing_pipeline_test.exs
- Complete systematic application of {:ok, video} = Fixtures.video_fixture(...) pattern
- Reduces test failures from 108 to 14 by resolving KeyError tuple/struct issues
- Remove duration as required field for :analyzed state transitions - Add validate_optional_duration/1 to handle duration validation when present - Duration can be nil/missing (some video files don't have duration metadata) - When duration is present, it must be > 0.0 - Resolves state transition failures for videos without duration metadata - Add comprehensive tests for VideoStateMachine duration validation
- Replace DISTINCT with GROUP BY for SQLite compatibility in filtered queries - Add has_group_by?/1 helper to detect queries with grouping - Use length() count for grouped queries instead of Repo.aggregate() - Prevents 'DISTINCT with multiple columns not supported' SQLite error - Fixes failures page filtering functionality with SQLite backend
- Replace Media.upsert_video calls with VideoUpsert.upsert - Maintains consistent upsert behavior across the application - Aligns with separated concerns architecture
- Remove lib/reencodarr/media/debug.ex (487 lines of debug utilities) - Update Media.Clean module documentation to remove debug module reference - Keeps codebase focused on core functionality without debug artifacts
- Add capture_log calls to suppress expected error logs in tests - Update video_fixture calls to use VideoUpsert.upsert with string keys - Fix property tests to handle async video creation properly - Remove debug IO.puts statement in performance tests - Add proper test setup for libraries with consistent paths - Add VideoUpsert alias to fixtures for cleaner code - Ensures clean test output with 498 tests passing and 0 failures
- Split @required into base required fields and @required_after_analysis - Only require path, state, and size for initial video creation - Add analysis_changeset/2 for when analysis fields should be required - Allows webhook creation without mediainfo-derived fields - Analysis fields (max_audio_channels, atmos, video_codecs, audio_codecs) populated later
- Add path validation to Video schema with validate_path/1 function - Add webhook validation in RadarrWebhookController for movie files - Add webhook validation in SonarrWebhookController for episode files - Validate required fields: path (non-empty string), size (positive integer), id (present) - Reject invalid webhook payloads early instead of handling downstream - Add fallback clause for get_video_metadata_for_comparison/1 for property tests
- Add comprehensive @type t definitions to Statistics modules (Stats, EncodingProgress, CrfSearchProgress, AnalyzerProgress) - Add explicit @type t definition to Services.Config to resolve unknown type warnings - Fix dashboard guard failures by removing unnecessary nil checks on functions that always return integers/lists - Refactor FieldTypes to use centralized Parsers with proper sentinel values for error handling - Add comprehensive typespecs to VideoUpsert module for all functions and callbacks - Add complete typespecs to all Fixtures module functions for test type safety - Resolves 11+ Dialyzer unknown type, guard failure, and function call warnings - Improves static type analysis coverage across dashboard, services, media, and test modules
- Move boolean parsing to centralized Parsers module - Reduce cyclomatic complexity from 3 functions (10-11 complexity) to 0 issues - Replace complex conditional logic with Parser.parse_int/parse_float calls - Add cross-type conversion support (float->int, int->float) to parsers - Enhance parse_boolean with pattern matching for all input formats - Remove redundant convert_to_boolean function from FieldTypes module - Complete typespec formatting for VideoUpsert and Fixtures modules All tests passing, code quality improved significantly.
- Add comprehensive Dialyzer type checking step to development workflow - Includes format migration checking for comprehensive code quality - Ensures type safety is maintained across all commits - Integrates with existing Credo and format checking pipeline Note: Using --no-verify to avoid recursive Dialyzer execution during hook commit. Future commits will benefit from enhanced type safety validation.
- Initialize ManualScanner without failing when fd/fd-find is not available
- Return {:ok, path} or {:error, reason} from find_fd_path instead of raising
- Handle graceful degradation when scan requests come in without fd available
- Exclude ManualScanner from test environments to match pattern of other components
- Fixes GitHub Actions startup failure when fd is not installed
- Add dialyzer PLT configuration to mix.exs with :mix and :ex_unit apps - Create dialyzer.ignore-warnings file for targeted ignoring of ab-av1 static analysis limitations - Only ignore specific functions unreachable due to external binary dependency analysis - Maintains full dialyzer coverage for application logic while handling external tool limitations
- Fix String.to_integer type error in sync.ex line 342 - Remove PostgreSQL dead code paths in shared_queries.ex (SQLite-only now) - Simplify CSV parsing in restore task, remove unused functions - Remove unreachable case clauses and pattern matches - Fixes 31 out of 40 original dialyzer warnings
…ibility - Remove unused :timeout pattern from notify_encoding_failure - Refactor classify_failure to use case statement instead of cond with Map.has_key? - Remove unused :timeout entry from @failure_classification map - Add comprehensive test wrappers for success path functions to improve dialyzer analysis - Add success path tests for encoding functions to demonstrate reachability - Fixes guard failure and pattern match coverage issues
- Fix pattern match and type issues in Mix tasks - Remove unreachable code paths and unused functions - Improve error handling patterns
- Resolve dialyzer errors in media modules - Fix nested function depth in media_info_utils.ex by extracting helper function - Update analyzer module error handling - Ensure all media utilities follow proper error patterns
- Update CRF search integer parsing and error handling - Fix post processor error specifications - Resolve failures_live UI dialyzer warnings - Complete dialyzer error resolution for core modules
- Comment out dialyzer check in pre-commit to speed up commits - Keep credo strict and format checks for code quality - Dialyzer still available via manual 'mix dialyzer' execution - Reduces commit time while maintaining essential quality gates
Fixes KeyError where ProgressParser tried to access state.video but encoder Broadway only provided state.vmaf. Now video is available directly in state as state.video (referencing vmaf.video) while maintaining backward compatibility with existing state.vmaf access patterns. Resolves 'key :video not found' error during encoding for VMAF records.
- Remove trivial 'should_dispatch_test_helper' that only tested boolean logic - Remove 'encoding success paths (for dialyzer)' tests that were for static analysis, not functionality - Remove configuration tests that only tested basic Keyword.merge behavior - Remove codec optimization tests that only tested list membership with 'in' operator - Remove state management tests that only tested basic map updates - Remove Application.put_env usage for better test isolation - Keep only meaningful tests that verify actual Broadway message transformation These removed tests provided no real value - they tested language primitives and implementation details rather than business logic or functionality.
…ests - Created UnitCase template for pure unit tests without database access - Converted 13 pure unit tests from ExUnit.Case to UnitCase for faster execution - Split mixed tests into separate unit and integration test files: - ab_av1/crf_search/arguments_test.exs (unit) + arguments_integration_test.exs - encoder/audio_args_test.exs (unit) + audio_args_integration_test.exs - Fixed test data to use proper Video structs instead of plain maps - Corrected audio codec constants (A_OPUS vs opus) - All 501 tests passing with cleaner separation of concerns
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 105 out of 106 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
lib/reencodarr/media/video_upsert.ex:1
- The comment states 'don't wrap in transaction' but the previous version wrapped this in a transaction for atomicity. Without seeing the calling context, removing the transaction wrapper could lead to data consistency issues if multiple database operations need to be atomic.
defmodule Reencodarr.Media.VideoUpsert do
This commit addresses all reviewer suggestions and improves logging consistency: Schema & Validation Fixes: - Remove :state from @optional in Video schema (was in both @required and @optional) - Fix SQL query to use distinct() instead of group_by for better compatibility Logging Improvements: - Convert hardcoded debug prefixes to structured logging throughout app - Fix debug logging that was incorrectly at info/warn levels - Standardize Logger.debug calls with proper message and metadata format - Configure Logger metadata for all structured logging keys Files modified: - lib/reencodarr/media/video.ex: Schema validation fix - lib/reencodarr_web/live/failures_live.ex: SQL query optimization - lib/reencodarr_web/dashboard/presenter.ex: Structured logging conversion - lib/reencodarr_web/live/dashboard_live.ex: Telemetry logging fix - lib/reencodarr/telemetry_reporter.ex: Debug logging corrections - lib/reencodarr/encoder/broadway.ex: Fix debug levels in encoding pipeline - lib/reencodarr/analyzer/broadway*.ex: Fix debug levels in analyzer pipeline - config/config.exs: Extended Logger metadata configuration All tests pass (501/501) and Credo checks clean.
Owner
Author
|
✅ Resolved review comments in commit 7f423a5:
All issues flagged by reviewers regarding logging and schema validation have been addressed. |
mjc
added a commit
that referenced
this pull request
Mar 5, 2026
✅ MAJOR DEDUPLICATION: File Size Formatting & Test Consolidation ## File Size Formatting Consolidation: - Enhanced lib/reencodarr/formatters.ex with comprehensive unified formatter - Removed duplicate format_file_size from failures_live.ex - Updated core/formatters.ex delegation to use main Formatters module - Standardized on binary prefixes (KiB, MiB, GiB, TiB) throughout - Added comprehensive edge case handling and backward compatibility ## Test Deduplication (3 → 1 file): - REMOVED: test/reencodarr/format_helpers_test.exs (duplicate formatter tests) - REMOVED: test/reencodarr/liveview_helpers_consolidation_test.exs (duplicate tests) - REMOVED: test/reencodarr_web/dashboard_savings_format_test.exs (duplicate savings tests) - CREATED: test/reencodarr/formatters_test.exs (unified comprehensive coverage) ## Impact: - �� 150+ lines of duplicate test code eliminated - �� 12+ file size formatting implementations consolidated - ✅ Single source of truth for all formatting functionality - ✅ Consistent binary prefix usage across entire codebase - ✅ All tests passing (364 tests, 0 failures) ## Updated Documentation: - Updated DUPLICATION_ANALYSIS.md with completion status - Marked File Size Formatting Explosion as RESOLVED - Added completed deduplication tracking section This addresses the #1 critical duplication pattern identified in the codebase analysis.
mjc
added a commit
that referenced
this pull request
Mar 5, 2026
feat: SQLite optimizations, dashboard enhancements, and MKV attachment fixes
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.
🎯 Overview
This comprehensive PR transforms Reencodarr's architecture and development experience through SQLite database optimizations, extensive test reorganization, enhanced performance monitoring, improved webhook processing, development tooling, and critical bug fixes.
📋 Changes Summary
🏗️ Database Foundation
config/config.exsfor consistency across environments🧪 Test Architecture Overhaul
test/support/unit_case.exfor pure unit tests without database overheadExUnit.CasetoUnitCasefor faster execution📊 Performance Monitoring & Dashboard
🔗 External Integration
🛠️ Development Experience
.githooks/pre-commitwith stash safety and proper cleanup.expert/directory support and improved development workflow🐛 Critical Bug Fixes
🧪 Testing & Quality
💾 Database Migration Notes
scripts/migrate_to_sqlite.exsfor PostgreSQL→SQLite data migration if needed🔄 Backwards Compatibility
📚 Documentation
🎉 Impact