Pre-refactor checkpoint: stream_organizer v4.8

Saving state before major refactoring of stream_organizer.js.
Current version is functional - this commit allows safe rollback.

Local/agent_notes/code_review_report.md

@@ -0,0 +1,133 @@
+# Tdarr Plugin Stack Code Review
+
+**Date:** 2025-12-15
+**Reviewer:** Antigravity Agent
+**Scope:** `/Local/*.js` plugins
+
+---
+
+## 1. Stack Architecture Overview
+
+The plugin stack operates in a **sequential re-queue model**. Each plugin that modifies the file triggers a `reQueueAfter: true`, causing Tdarr to process the output and restart the stack from the beginning with the new file.
+
+**Current Order:**
+1. `misc_fixes` (Container/Remux/Clean)
+2. `stream_organizer` (Subtitle Extraction/Reorder)
+3. `combined_audio_standardizer` (Audio Transcode/Downmix)
+4. `av1_svt_converter` (Video Transcode)
+
+### 🚨 Architectural Risks & Findings
+*   **High I/O Overhead:** This stack can potentially trigger **4 separate transcode/remux cycles** per file.
+    *   Pass 1: Remux to MKV (misc_fixes)
+    *   Pass 2: Reorder/Extract Subtitles (stream_organizer)
+    *   Pass 3: Audio Transcode (audio_standardizer)
+    *   Pass 4: Video Transcode (av1_converter)
+    *   *Recommendation:* Consider combining logic where possible, or accepting high I/O for modularity.
+*   **Race Conditions:** `stream_organizer` handles CC extraction via mostly atomic locks, but file existence checks in Tdarr's distributed environment are always tricky. The current `reQueueAfter` logic relies heavily on "state convergence" (eventually the file meets criteria).
+
+---
+
+## 2. Individual Plugin Analysis
+
+### A. Tdarr_Plugin_misc_fixes.js (v2.8)
+
+**Overview:** Handles container standardization, stream cleaning, and TS fixes.
+
+**Strengths:**
+*   **Correct Logic Flow:** Checks for "work already done" (e.g., `currentContainer !== targetContainer`, `firstStreamIsVideo`) to prevent infinite loops.
+*   **Robust Skip Logic:** Correctly identifies unfixable ISO/TS files early.
+
+**Issues & Improvements:**
+1.  **Complexity/Refactoring:** The `plugin` function is becoming monolithic.
+    *   *Suggestion:* Extract `analyzeStreams` and `buildFFmpegCommand` into helper functions.
+2.  **Hardcoded Lists:** `brokenTypes`, `image codecs` are defined inside the function.
+    *   *Fix:* Move `const` definitions (like `BROKEN_TYPES`, `IMAGE_CODECS`) to top-level scope for readability and memory efficiency.
+3.  **Variable Shadowing:** `inputs` is reassigned (`inputs = lib.loadDefaultValues...`). Ideally, use `const settings = ...` to avoid mutating arguments.
+
+### B. Tdarr_Plugin_stream_organizer.js (v4.8)
+
+**Overview:** Manages subtitles, language ordering, and extraction.
+
+**Strengths:**
+*   **Sanitization:** Strong input/filename sanitization (`sanitizeFilename`, `sanitizeForShell`).
+*   **Loop Protection:** Excellent use of `MAX_EXTRACTION_ATTEMPTS` and `extractionAttempts` map (though memory-only).
+*   **Robustness:** `fileExistsRobust` wrapper helps with filesystem flakes.
+
+**Issues & Improvements:**
+1.  **Massive Function Size:** The `plugin` function is ~500 lines. It violates Single Responsibility Principle.
+    *   *Critical Refactor:* Move stream analysis, extraction logic, and command building into separate functions: `getReorderedStreams()`, `processSubtitles()`, `buildFFmpegArgs()`.
+2.  **Redundant Logic:** `isEnglishStream` is used in partitioning and mapping loops separately.
+3.  **CC Extraction Lock:** The lock file mechanism (`.lock`) is decent but relies on `process.pid`. If a node crashes hard, the lock remains.
+    *   *Recommendation:* Add a "stale lock" check (e.g., if lock file > 1 hour old, ignore/delete it).
+4.  **Efficiency:** The `partitionStreams` logic iterates arrays multiple times.
+
+### C. Tdarr_Plugin_combined_audio_standardizer.js (v1.13)
+
+**Overview:** Complex audio mapping, downmixing, and transcoding.
+
+**Strengths:**
+*   **Modular Helpers:** `buildCodecArgs`, `buildDownmixArgs`, `calculateBitrate` are well-separated. Good code structure.
+*   **Explicit Mapping:** Correctly handles attachments via `streamMap` construction (prevents the "muxing overhead 400%" issues).
+
+**Issues & Improvements:**
+1.  **Complex Conditionals:** The `needsTranscoding` logic is a bit nested.
+2.  **Downmix Logic Risk:** `buildDownmixArgs` assumes the source stream is compatible with the `downmix` filter. Usually safe, but edge cases exist.
+3.  **Attachment Handling:** It maps `0:t` copies, but `misc_fixes` might have stripped images.
+    *   *Check:* If `misc_fixes` runs first, it removes images. `audio_standardizer` won't see them in `file.ffProbeData` (sourced from Tdarr DB).
+    *   *Risk:* If Tdarr DB is stale (scan didn't happen after misc_fixes?), `combined_audio` might try to map non-existent streams.
+    *   *Mitigation:* `reQueueAfter` usually forces a rescan, so this should be safe.
+
+### D. Tdarr_Plugin_av1_svt_converter.js (v2.22)
+
+**Overview:** AV1 video encoding.
+
+**Strengths:**
+*   **Modern AV1 Handling:** Good use of SVT-AV1 parameters (SCD, TF, etc.).
+*   **Resolution Awareness:** Smart CRF adjustment logic based on resolution.
+*   **Input Handling:** Explicit checks for HDR/10-bit.
+
+**Issues & Improvements:**
+1.  **Argument Injection Risk (Low):** `svtParams` is constructed from inputs. While inputs are sanitized (stripped stars), strict type validation would be better before injection.
+2.  **Parsing Logic:** `resolutionMap` is hardcoded.
+3.  **Bitrate Strategy:** The `target_bitrate_strategy` logic is complex and relies on accurate source bitrate detection, which isn't always available in `ffProbeData`.
+    *   *Suggestion:* Add fallback if `bit_rate` is missing/NaN (currently defaults to safe uncapped, which is acceptable).
+
+---
+
+## 3. General Best Practice Violations
+
+1.  **Shared Helpers Duplication:**
+    *   `stripStar` is defined in EVERY plugin.
+    *   `sanitizeForShell` is in multiple plugins.
+    *   *Fix:* You have a `lib/sanitization.js` (referenced in chat history), but plugins currently duplicate this code. They should `require` the shared library if Tdarr environment permits, OR (if Tdarr requires self-contained plugins) this duplication is a necessary evil.
+    *   *Observation:* Plugins currently require `../methods/lib` (Tdarr internal). Custom libs in `/Local` might not be reliably accessible across nodes unless explicitly distributed.
+
+2.  **Magic Numbers:**
+    *   `MAX_EXTRACTION_ATTEMPTS = 3`
+    *   `MIN_SUBTITLE_FILE_SIZE = 100`
+    *   Defined as constants in some files, literals in others. Standardize.
+
+3.  **Error Handling Patterns:**
+    *   Most plugins use `response.processFile = false` + `infoLog` on error. This is good Tdarr practice (don't crash the node).
+
+---
+
+## 4. Recommendations & Refactoring Plan
+
+### Priority 1: Safety & Stability (Immediate)
+*   **Stale Lock Cleanup:** Implement stale lock check in `stream_organizer` (CC extraction).
+*   **Argument Validation:** Strengthen input validation in `av1_svt_converter` to ensure `svt-params` injection is perfectly safe.
+
+### Priority 2: Code Quality (Short-term)
+*   **De-duplication:** If Tdarr allows, strictly enforce using a shared `utils.js` for `stripStar`, `sanitizeFilename`, etc.
+*   **Modularization:** Refactor `stream_organizer.js` to break up the 500-line `plugin` function.
+
+### Priority 3: Architecture (Long-term)
+*   **Combine Passes:** Investigate merging `misc_fixes` and `stream_organizer` logic?
+    *   *Counter-argument:* Keeping them separate is better for maintenance.
+    *   *Alternative:* Use Tdarr's "Flows" (if upgrading to Tdarr V2 flows) or accept the I/O cost for robustness.
+
+## 5. Conclusion
+The plugins are currently **FUNCTIONAL and SAFE** (after recent fixes). The code quality is generally high but suffers from "script creep" where functions have grown too large. Logic for infinite loop prevention is verified in place.
+
+**No immediate code changes required for safety**, but refactoring `stream_organizer` is highly recommended for maintainability.

agent_notes/stream_organizer_refactor_plan.md

@@ -0,0 +1,92 @@
+# Stream Organizer Refactoring Plan
+
+## Current State Analysis
+
+**File:** `Tdarr_Plugin_stream_organizer.js` (v4.8)
+**Total Lines:** 777
+**Main Function Lines:** ~500 (lines 251-772)
+
+### Problems Identified
+1. **Monolithic Function:** The `plugin` function handles too many responsibilities
+2. **Deep Nesting:** Multiple levels of conditionals and loops
+3. **Difficult Testing:** Cannot test sub-components in isolation
+4. **Hard to Maintain:** Changes require understanding entire 500-line function
+
+## Refactoring Strategy
+
+### Phase 1: Extract Stream Analysis Logic
+**New Functions:**
+- `analyzeStreams(file)` → Returns categorized streams (video, audio, subtitle, other)
+- `reorderStreamsByLanguage(streams, languageCodes, includeAudio, includeSubtitles)` → Returns reordered streams
+- `detectConversionNeeds(subtitleStreams, inputs)` → Returns conversion analysis
+
+### Phase 2: Extract Subtitle Processing
+**New Functions:**
+- `processSubtitleExtraction(subtitleStreams, inputs, otherArguments, fs, path)` → Returns extraction command + metadata
+- `processCCExtraction(subtitleStreams, inputs, otherArguments, fs)` → Returns CC extraction plan
+- `buildSubtitleMapping(reorderedStreams, inputs, customEnglishCodes)` → Returns subtitle map commands
+
+### Phase 3: Extract FFmpeg Command Building
+**New Functions:**
+- `buildFFmpegCommand(analysis, inputs)` → Main orchestrator
+- `buildBaseMapping(reorderedStreams, inputs)` → Returns base -map commands
+- `buildCodecArgs(includedSubtitles, inputs)` → Returns codec arguments
+- `buildDispositionArgs(audioIdx, subIdx)` → Returns default flag arguments
+
+### Phase 4: Simplify Main Plugin Function
+**New Structure:**
+```javascript
+const plugin = (file, librarySettings, inputs, otherArguments) => {
+  const { lib, fs, path } = initDependencies();
+  const response = initResponse(file);
+  
+  try {
+    inputs = validateAndSanitizeInputs(inputs, lib, details);
+    
+    const analysis = analyzeStreams(file, inputs, otherArguments, fs, path);
+    
+    if (!needsProcessing(analysis)) {
+      return skipResponse(analysis.message);
+    }
+    
+    const command = buildFFmpegCommand(analysis, inputs);
+    
+    return buildSuccessResponse(command, analysis, inputs);
+  } catch (error) {
+    return buildErrorResponse(error, file);
+  }
+};
+```
+
+## Implementation Checklist
+
+- [ ] Create helper module structure at top of file
+- [ ] Extract `analyzeStreams()` family
+- [ ] Extract subtitle processing functions
+- [ ] Extract FFmpeg command builders
+- [ ] Refactor main `plugin()` function
+- [ ] Test equivalence (commands should be identical)
+- [ ] Update version to 4.9
+- [ ] Add inline documentation
+
+## Benefits
+
+1. **Testability:** Each function can be unit tested
+2. **Readability:** Functions have clear, single purposes
+3. **Maintainability:** Changes are localized to specific functions
+4. **Debuggability:** Stack traces show which component failed
+5. **Reusability:** Logic can be shared/adapted for other plugins
+
+## Risk Mitigation
+
+- **No Behavior Changes:** All existing logic preserved exactly
+- **Incremental Approach:** Refactor in phases, test after each
+- **Version Bump:** Clear signal this is a refactored version
+- **Git History:** Commit shows before/after for rollback
+
+## Next Steps
+
+1. Create backup/branch point
+2. Begin Phase 1: Stream analysis extraction
+3. Validate output equivalence with test files
+4. Continue with subsequent phases