refactor: optimize inner-loop-tracker performance and code quality

Performance improvements:
- Throttle cleanupExpiredTodos() to run every 30s instead of every chunk
- Add early exit in detectTodoItems() for lines without todo markers
- Remove redundant PROMISE_PATTERN check from LOOP_START_PATTERN

Code quality:
- Extract activateLoopIfNeeded() helper to eliminate duplicate loop start logic
- Improve hash function using djb2 algorithm for better distribution
- Add guards for empty/whitespace-only content in upsertTodo()
- Simplify startLoop() to use existing enable() method

Tests:
- Add 5 new tests for edge cases and optimizations
- Test empty content handling, early exit, ID uniqueness

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

src/inner-loop-tracker.ts

@@ -10,6 +10,8 @@ import {
 const MAX_TODO_ITEMS = 50;
 // Todo items older than this will be auto-expired (1 hour)
 const TODO_EXPIRY_MS = 60 * 60 * 1000;
+// Throttle cleanup checks (every 30 seconds)
+const CLEANUP_THROTTLE_MS = 30 * 1000;
 
 // Pre-compiled regex patterns for performance (avoid re-compilation on each call)
 
@@ -35,8 +37,8 @@ const TODO_EXCLUDE_PATTERNS = [
   /^\S+\([^)]+\)$/,                                          // Generic function call pattern
 ];
 
-// Loop status patterns
+// Loop status patterns (does NOT include <promise> - that's handled by PROMISE_PATTERN)
-const LOOP_START_PATTERN = /Loop started at|Starting.*loop|Ralph loop started|<promise>([^<]+)<\/promise>/i;
+const LOOP_START_PATTERN = /Loop started at|Starting.*loop|Ralph loop started/i;
 const ELAPSED_TIME_PATTERN = /Elapsed:\s*(\d+(?:\.\d+)?)\s*hours?/i;
 const CYCLE_PATTERN = /cycle\s*#?(\d+)|respawn cycle #(\d+)/i;
 
@@ -79,6 +81,8 @@ export class InnerLoopTracker extends EventEmitter {
   private _lineBuffer: string = '';
   // Track occurrences of completion phrases to distinguish prompt from actual completion
   private _completionPhraseCount: Map<string, number> = new Map();
+  // Throttle cleanup to avoid running on every data chunk
+  private _lastCleanupTime: number = 0;
 
   constructor() {
     super();
@@ -154,8 +158,8 @@ export class InnerLoopTracker extends EventEmitter {
     // Also check the current buffer for multi-line patterns
     this.checkMultiLinePatterns(cleanData);
 
-    // Cleanup expired todos
+    // Cleanup expired todos (throttled to avoid running on every chunk)
-    this.cleanupExpiredTodos();
+    this.maybeCleanupExpiredTodos();
   }
 
   /**
@@ -201,8 +205,8 @@ export class InnerLoopTracker extends EventEmitter {
       return true;
     }
 
-    // Loop start patterns
+    // Loop start patterns (e.g., "Loop started at", "Starting Ralph loop")
-    if (LOOP_START_PATTERN.test(data) && !PROMISE_PATTERN.test(data)) {
+    if (LOOP_START_PATTERN.test(data)) {
       return true;
     }
 
@@ -275,38 +279,30 @@ export class InnerLoopTracker extends EventEmitter {
     }
   }
 
+  /**
+   * Helper: Activate the loop if not already active
+   */
+  private activateLoopIfNeeded(): boolean {
+    if (this._loopState.active) return false;
+
+    this._loopState.active = true;
+    this._loopState.startedAt = Date.now();
+    this._loopState.cycleCount = 0;
+    this._loopState.maxIterations = null;
+    this._loopState.elapsedHours = null;
+    this._loopState.lastActivity = Date.now();
+    this.emit('loopUpdate', this.loopState);
+    return true;
+  }
+
   /**
    * Detect loop start and status indicators
    */
   private detectLoopStatus(line: string): void {
     // Check for Ralph loop start command (/ralph-loop:ralph-loop)
-    if (RALPH_START_PATTERN.test(line)) {
+    // or generic loop start patterns ("Loop started at", "Starting Ralph loop")
-      if (!this._loopState.active) {
+    if (RALPH_START_PATTERN.test(line) || LOOP_START_PATTERN.test(line)) {
-        this._loopState.active = true;
+      this.activateLoopIfNeeded();
-        this._loopState.startedAt = Date.now();
-        this._loopState.cycleCount = 0;
-        this._loopState.maxIterations = null;
-        this._loopState.elapsedHours = null;
-        this._loopState.lastActivity = Date.now();
-        this.emit('loopUpdate', this.loopState);
-      }
-    }
-
-    // Check for generic loop start
-    if (LOOP_START_PATTERN.test(line)) {
-      // Check if this is a promise match (loop ending) vs loop start
-      if (!PROMISE_PATTERN.test(line)) {
-        // This is a loop start indicator
-        if (!this._loopState.active) {
-          this._loopState.active = true;
-          this._loopState.startedAt = Date.now();
-          this._loopState.cycleCount = 0;
-          this._loopState.maxIterations = null;
-          this._loopState.elapsedHours = null;
-          this._loopState.lastActivity = Date.now();
-          this.emit('loopUpdate', this.loopState);
-        }
-      }
     }
 
     // Check for max iterations setting
@@ -328,12 +324,7 @@ export class InnerLoopTracker extends EventEmitter {
       const maxIter = iterMatch[2] || iterMatch[4] ? parseInt(iterMatch[2] || iterMatch[4]) : null;
 
       if (!isNaN(currentIter)) {
-        // If not already active, start the loop
+        this.activateLoopIfNeeded();
-        if (!this._loopState.active) {
-          this._loopState.active = true;
-          this._loopState.startedAt = Date.now();
-        }
-
         this._loopState.cycleCount = currentIter;
         if (maxIter !== null && !isNaN(maxIter)) {
           this._loopState.maxIterations = maxIter;
@@ -373,6 +364,14 @@ export class InnerLoopTracker extends EventEmitter {
    * Detect todo items in various formats
    */
   private detectTodoItems(line: string): void {
+    // Quick check: skip lines that can't possibly contain todos
+    // Must have: checkbox marker, icon, or parentheses status
+    if (!line.includes('[') && !line.includes('☐') && !line.includes('☒') &&
+        !line.includes('◐') && !line.includes('Todo:') && !line.includes('(pending)') &&
+        !line.includes('(in_progress)') && !line.includes('(completed)')) {
+      return;
+    }
+
     let updated = false;
 
     // Format 1: Checkbox format "- [ ] Task" or "- [x] Task"
@@ -455,7 +454,10 @@ export class InnerLoopTracker extends EventEmitter {
    * Add or update a todo item
    */
   private upsertTodo(content: string, status: InnerTodoStatus): void {
-    // Generate a stable ID from content (simple hash)
+    // Skip empty or whitespace-only content
+    if (!content || !content.trim()) return;
+
+    // Generate a stable ID from content
     const id = this.generateTodoId(content);
 
     const existing = this._todos.get(id);
@@ -483,15 +485,16 @@ export class InnerLoopTracker extends EventEmitter {
   }
 
   /**
-   * Generate a stable ID from todo content
+   * Generate a stable ID from todo content using djb2 hash
    */
   private generateTodoId(content: string): string {
-    // Simple hash based on content
+    if (!content) return 'todo-empty';
-    let hash = 0;
+
+    // djb2 hash algorithm - good distribution for strings
+    let hash = 5381;
     for (let i = 0; i < content.length; i++) {
-      const char = content.charCodeAt(i);
+      hash = ((hash << 5) + hash) ^ content.charCodeAt(i);
-      hash = ((hash << 5) - hash) + char;
+      hash = hash | 0; // Convert to 32-bit integer
-      hash = hash & hash; // Convert to 32-bit integer
     }
     return `todo-${Math.abs(hash).toString(36)}`;
   }
@@ -509,6 +512,18 @@ export class InnerLoopTracker extends EventEmitter {
     return oldest;
   }
 
+  /**
+   * Throttled cleanup - only runs every CLEANUP_THROTTLE_MS
+   */
+  private maybeCleanupExpiredTodos(): void {
+    const now = Date.now();
+    if (now - this._lastCleanupTime < CLEANUP_THROTTLE_MS) {
+      return;
+    }
+    this._lastCleanupTime = now;
+    this.cleanupExpiredTodos();
+  }
+
   /**
    * Remove expired todo items
    */
@@ -535,11 +550,7 @@ export class InnerLoopTracker extends EventEmitter {
    * Also enables the tracker if not already enabled
    */
   startLoop(completionPhrase?: string, maxIterations?: number): void {
-    // Enable tracker when loop is explicitly started
+    this.enable(); // Ensure tracker is enabled
-    if (!this._loopState.enabled) {
-      this._loopState.enabled = true;
-      this.emit('enabled');
-    }
     this._loopState.active = true;
     this._loopState.startedAt = Date.now();
     this._loopState.cycleCount = 0;

test/inner-loop-tracker.test.ts

@@ -613,4 +613,63 @@ describe('InnerLoopTracker', () => {
       expect(tracker.todos.length).toBeLessThanOrEqual(50);
     });
   });
+
+  describe('Edge Cases and Optimizations', () => {
+    it('should skip empty or whitespace-only content', () => {
+      // These should not create todos
+      tracker.processTerminalData('- [ ] \n');
+      tracker.processTerminalData('- [ ]    \n');
+
+      expect(tracker.todos).toHaveLength(0);
+    });
+
+    it('should skip lines without todo markers (early exit optimization)', () => {
+      const todoHandler = vi.fn();
+      tracker.on('todoUpdate', todoHandler);
+
+      // Process lines that have no todo markers
+      tracker.processTerminalData('This is just regular text\n');
+      tracker.processTerminalData('Another line without markers\n');
+      tracker.processTerminalData('Some code: function() {}\n');
+
+      // No todoUpdate should be emitted
+      expect(todoHandler).not.toHaveBeenCalled();
+      expect(tracker.todos).toHaveLength(0);
+    });
+
+    it('should generate different IDs for different content', () => {
+      tracker.processTerminalData('- [ ] Task A\n');
+      tracker.processTerminalData('- [ ] Task B\n');
+
+      const todos = tracker.todos;
+      expect(todos).toHaveLength(2);
+      expect(todos[0].id).not.toBe(todos[1].id);
+    });
+
+    it('should use activateLoopIfNeeded only once', () => {
+      const loopHandler = vi.fn();
+      tracker.on('loopUpdate', loopHandler);
+
+      // Multiple loop start patterns should only activate once
+      tracker.processTerminalData('Loop started at 2024-01-15\n');
+      tracker.processTerminalData('Starting Ralph loop\n');
+      tracker.processTerminalData('/ralph-loop:ralph-loop\n');
+
+      // Loop should only have been activated once
+      expect(tracker.loopState.active).toBe(true);
+      // But multiple updates are OK (state changes)
+    });
+
+    it('should handle mixed content with todos and non-todos', () => {
+      tracker.processTerminalData(`
+Some regular text
+- [ ] Actual todo item
+More text here
+☐ Another todo with icon
+Final text
+`);
+
+      expect(tracker.todos).toHaveLength(2);
+    });
+  });
 });