Merge pull request 'Fixes #11: Memory management enhancements' (#12) from issue-11-memory-management into main

Reviewed-on: #12

__tests__/memoryManagement.test.js

@@ -0,0 +1,240 @@
+/**
+ * Memory Management Tests
+ * Tests DOM cache invalidation, event listener cleanup, and debug log management
+ */
+
+describe('DOM Cache Invalidation', () => {
+    test('clearDomCache sets all jQuery references to null', () => {
+        // Simulate the clearDomCache logic
+        const domCache = {
+            $panelContainer: { isPanel: true },
+            $userStatsContainer: { isStats: true },
+            $infoBoxContainer: { isInfoBox: true },
+            $thoughtsContainer: { isThoughts: true },
+            $inventoryContainer: { isInventory: true },
+            $questsContainer: { isQuests: true },
+            $musicPlayerContainer: { isMusic: true },
+            $equipmentContainer: { isEquipment: true }
+        };
+
+        // Simulate clearDomCache
+        domCache.$panelContainer = null;
+        domCache.$userStatsContainer = null;
+        domCache.$infoBoxContainer = null;
+        domCache.$thoughtsContainer = null;
+        domCache.$inventoryContainer = null;
+        domCache.$questsContainer = null;
+        domCache.$musicPlayerContainer = null;
+        domCache.$equipmentContainer = null;
+
+        // Verify all are null
+        expect(domCache.$panelContainer).toBeNull();
+        expect(domCache.$userStatsContainer).toBeNull();
+        expect(domCache.$infoBoxContainer).toBeNull();
+        expect(domCache.$thoughtsContainer).toBeNull();
+        expect(domCache.$inventoryContainer).toBeNull();
+        expect(domCache.$questsContainer).toBeNull();
+        expect(domCache.$musicPlayerContainer).toBeNull();
+        expect(domCache.$equipmentContainer).toBeNull();
+    });
+
+    test('clearDomCache is idempotent (safe to call multiple times)', () => {
+        const cache = { $panelContainer: null };
+
+        // Call clear twice - should not throw
+        cache.$panelContainer = null;
+        cache.$panelContainer = null;
+
+        expect(cache.$panelContainer).toBeNull();
+    });
+});
+
+describe('Debug Log Memory Management', () => {
+    test('debugLogs array is capped at 100 entries', () => {
+        const debugLogs = [];
+        const MAX_LOGS = 100;
+
+        // Add 150 entries
+        for (let i = 0; i < 150; i++) {
+            debugLogs.push({ timestamp: new Date().toISOString(), message: `Log ${i}` });
+            if (debugLogs.length > MAX_LOGS) {
+                debugLogs.shift();
+            }
+        }
+
+        expect(debugLogs.length).toBe(100);
+        expect(debugLogs[0].message).toBe('Log 50'); // First 50 were shifted out
+        expect(debugLogs[99].message).toBe('Log 149');
+    });
+
+    test('clearDebugLogs empties the array', () => {
+        const debugLogs = [
+            { timestamp: '2024-01-01T00:00:00Z', message: 'Log 1' },
+            { timestamp: '2024-01-01T00:00:01Z', message: 'Log 2' },
+            { timestamp: '2024-01-01T00:00:02Z', message: 'Log 3' }
+        ];
+
+        // Simulate clearDebugLogs: debugLogs.length = 0
+        debugLogs.length = 0;
+
+        expect(debugLogs.length).toBe(0);
+    });
+
+    test('debugLogs cleared on chat change prevents memory accumulation', () => {
+        const debugLogs = [];
+        const MAX_LOGS = 100;
+
+        // Simulate multiple chat sessions
+        for (let chat = 0; chat < 5; chat++) {
+            // Clear on chat change
+            debugLogs.length = 0;
+
+            // Add logs during this chat session
+            for (let i = 0; i < 30; i++) {
+                debugLogs.push({ timestamp: new Date().toISOString(), message: `Chat ${chat} Log ${i}` });
+                if (debugLogs.length > MAX_LOGS) {
+                    debugLogs.shift();
+                }
+            }
+        }
+
+        // After 5 chats with clearing, we should only have logs from the last chat
+        expect(debugLogs.length).toBe(30);
+        expect(debugLogs[0].message).toBe('Chat 4 Log 0');
+    });
+});
+
+describe('Event Listener Cleanup', () => {
+    test('registered handlers are tracked for cleanup', () => {
+        // Simulate the registeredHandlers Map from events.js
+        const registeredHandlers = new Map();
+
+        function trackHandler(eventType, handler) {
+            if (!registeredHandlers.has(eventType)) {
+                registeredHandlers.set(eventType, []);
+            }
+            registeredHandlers.get(eventType).push(handler);
+        }
+
+        const handler1 = () => {};
+        const handler2 = () => {};
+
+        trackHandler('MESSAGE_SENT', handler1);
+        trackHandler('MESSAGE_SENT', handler2);
+        trackHandler('CHAT_CHANGED', handler1);
+
+        expect(registeredHandlers.get('MESSAGE_SENT').length).toBe(2);
+        expect(registeredHandlers.get('CHAT_CHANGED').length).toBe(1);
+    });
+
+    test('unregisterAllEvents clears all tracked handlers', () => {
+        const registeredHandlers = new Map();
+
+        function trackHandler(eventType, handler) {
+            if (!registeredHandlers.has(eventType)) {
+                registeredHandlers.set(eventType, []);
+            }
+            registeredHandlers.get(eventType).push(handler);
+        }
+
+        trackHandler('MESSAGE_SENT', () => {});
+        trackHandler('CHAT_CHANGED', () => {});
+
+        // Simulate unregisterAllEvents
+        registeredHandlers.clear();
+
+        expect(registeredHandlers.size).toBe(0);
+    });
+
+    test('on() function tracks handlers for cleanup', () => {
+        const registeredHandlers = new Map();
+
+        // Simulate the on() function with tracking
+        function on(eventType, handler) {
+            // eventSource.on(eventType, handler); // Would call real event source
+            if (!registeredHandlers.has(eventType)) {
+                registeredHandlers.set(eventType, []);
+            }
+            registeredHandlers.get(eventType).push(handler);
+        }
+
+        const handler = () => {};
+        on('GENERATE_BEFORE_COMBINE_PROMPTS', handler);
+
+        expect(registeredHandlers.has('GENERATE_BEFORE_COMBINE_PROMPTS')).toBe(true);
+        expect(registeredHandlers.get('GENERATE_BEFORE_COMBINE_PROMPTS').includes(handler)).toBe(true);
+    });
+
+    test('once() function tracks handlers for cleanup', () => {
+        const registeredHandlers = new Map();
+
+        // Simulate the once() function with tracking
+        function once(eventType, handler) {
+            // eventSource.once(eventType, handler); // Would call real event source
+            if (!registeredHandlers.has(eventType)) {
+                registeredHandlers.set(eventType, []);
+            }
+            registeredHandlers.get(eventType).push(handler);
+        }
+
+        const handler = () => {};
+        once('TEXT_COMPLETION_SETTINGS_READY', handler);
+
+        expect(registeredHandlers.has('TEXT_COMPLETION_SETTINGS_READY')).toBe(true);
+        expect(registeredHandlers.get('TEXT_COMPLETION_SETTINGS_READY').includes(handler)).toBe(true);
+    });
+
+    test('multiple handlers for same event type are all tracked', () => {
+        const registeredHandlers = new Map();
+
+        function on(eventType, handler) {
+            if (!registeredHandlers.has(eventType)) {
+                registeredHandlers.set(eventType, []);
+            }
+            registeredHandlers.get(eventType).push(handler);
+        }
+
+        const handlers = [() => {}, () => {}, () => {}];
+        handlers.forEach(h => on('CHAT_CHANGED', h));
+
+        expect(registeredHandlers.get('CHAT_CHANGED').length).toBe(3);
+    });
+});
+
+describe('jQuery Event Handler Tracking', () => {
+    test('tracked jQuery handlers can be cleaned up', () => {
+        const jqueryEventHandlers = [];
+
+        function trackJQueryHandler(event, selector, handler) {
+            jqueryEventHandlers.push({ event, selector, handler });
+            return handler;
+        }
+
+        const handler1 = () => {};
+        const handler2 = () => {};
+
+        trackJQueryHandler('click', '.rpg-item-remove', handler1);
+        trackJQueryHandler('click', '.rpg-equip-btn', handler2);
+
+        expect(jqueryEventHandlers.length).toBe(2);
+
+        // Simulate cleanup
+        jqueryEventHandlers.length = 0;
+        expect(jqueryEventHandlers.length).toBe(0);
+    });
+
+    test('trackJQueryHandler returns the handler for chaining', () => {
+        const jqueryEventHandlers = [];
+
+        function trackJQueryHandler(event, selector, handler) {
+            jqueryEventHandlers.push({ event, selector, handler });
+            return handler;
+        }
+
+        const handler = () => 'test';
+        const returned = trackJQueryHandler('click', '.selector', handler);
+
+        expect(returned).toBe(handler);
+        expect(returned()).toBe('test');
+    });
+});

index.js

@@ -50,10 +50,12 @@ import {
     setEquipmentContainer,
     setQuestsContainer,
     setMusicPlayerContainer,
-    clearSessionAvatarPrompts
+    clearSessionAvatarPrompts,
+    clearDomCache,
+    clearDebugLogs
 } from './src/core/state.js';
 import { loadSettings, saveSettings, saveChatData, loadChatData, updateMessageSwipeData, commitTrackerDataFromPriorMessage } from './src/core/persistence.js';
-import { registerAllEvents } from './src/core/events.js';
+import { registerAllEvents, on as onEvent } from './src/core/events.js';
 import { addExtensionSettings } from './src/core/settingsPanel.js';
 
 // Generation & Parsing modules
@@ -233,6 +235,8 @@ async function initUI() {
     }
 
     // Cache UI elements using state setters
+    // Clear stale DOM references first to prevent memory leaks on panel rebuild
+    clearDomCache();
     setPanelContainer($('#rpg-companion-panel'));
     setUserStatsContainer($('#rpg-user-stats'));
     setInfoBoxContainer($('#rpg-info-box'));
@@ -372,7 +376,7 @@ jQuery(async () => {
                 [event_types.MESSAGE_RECEIVED]: onMessageReceived,
                 [event_types.GENERATION_STOPPED]: onGenerationEnded,
                 [event_types.GENERATION_ENDED]: onGenerationEnded,
-                [event_types.CHAT_CHANGED]: [onCharacterChanged, updatePersonaAvatar, restoreCheckpointOnLoad, clearSessionAvatarPrompts],
+                [event_types.CHAT_CHANGED]: [onCharacterChanged, updatePersonaAvatar, restoreCheckpointOnLoad, clearSessionAvatarPrompts, clearDebugLogs],
                 [event_types.CHAT_LOADED]: onChatLoaded,
                 [event_types.MESSAGE_DELETED]: onMessageDeleted,
                 [event_types.MESSAGE_SWIPE_DELETED]: onMessageDeleted,
@@ -381,7 +385,9 @@ jQuery(async () => {
                 [event_types.SETTINGS_UPDATED]: updatePersonaAvatar
             });
 
-            eventSource.on(event_types.CHARACTER_MESSAGE_RENDERED, (messageId) => {
+            // Use tracked event registration (onEvent) instead of direct eventSource.on()
+            // This ensures all handlers are tracked for cleanup by unregisterAllEvents()
+            onEvent(event_types.CHARACTER_MESSAGE_RENDERED, (messageId) => {
                 if (!extensionSettings.enabled) return;
                 const renderedMessage = chat[messageId];
                 if (renderedMessage && !renderedMessage.is_user && !renderedMessage.is_system) {
@@ -389,7 +395,7 @@ jQuery(async () => {
                 }
             });
 
-            eventSource.on(event_types.MESSAGE_UPDATED, (messageId) => {
+            onEvent(event_types.MESSAGE_UPDATED, (messageId) => {
                 if (!extensionSettings.enabled) return;
                 const updatedMessage = chat[messageId];
                 if (updatedMessage && !updatedMessage.is_user && !updatedMessage.is_system) {
@@ -397,7 +403,7 @@ jQuery(async () => {
                 }
             });
 
-            eventSource.on(event_types.MESSAGE_SWIPED, (messageIndex) => {
+            onEvent(event_types.MESSAGE_SWIPED, (messageIndex) => {
                 if (!extensionSettings.enabled) return;
                 const swipedMessage = chat[messageIndex];
                 if (swipedMessage && !swipedMessage.is_user && !swipedMessage.is_system) {
@@ -405,18 +411,18 @@ jQuery(async () => {
                 }
             });
 
-            eventSource.on(event_types.CHAT_CHANGED, () => {
+            onEvent(event_types.CHAT_CHANGED, () => {
                 clearThoughtBasedExpressionsCache();
                 setTimeout(() => onThoughtBasedExpressionsChatChanged(), 0);
             });
 
-            eventSource.on(event_types.MESSAGE_DELETED, () => {
+            onEvent(event_types.MESSAGE_DELETED, () => {
                 if (!extensionSettings.enabled) return;
                 clearThoughtBasedExpressionsCache();
                 setTimeout(() => onThoughtBasedExpressionsChatChanged(), 0);
             });
 
-            eventSource.on(event_types.MESSAGE_SWIPE_DELETED, () => {
+            onEvent(event_types.MESSAGE_SWIPE_DELETED, () => {
                 if (!extensionSettings.enabled) return;
                 clearThoughtBasedExpressionsCache();
                 setTimeout(() => onThoughtBasedExpressionsChatChanged(), 0);

src/core/events.js

@@ -12,6 +12,12 @@ import { eventSource, event_types } from '../../../../../../script.js';
  */
 export function on(eventType, handler) {
     eventSource.on(eventType, handler);
+
+    // Track for cleanup
+    if (!registeredHandlers.has(eventType)) {
+        registeredHandlers.set(eventType, []);
+    }
+    registeredHandlers.get(eventType).push(handler);
 }
 
 /**
@@ -21,6 +27,12 @@ export function on(eventType, handler) {
  */
 export function once(eventType, handler) {
     eventSource.once(eventType, handler);
+
+    // Track for cleanup (one-time handlers should also be tracked)
+    if (!registeredHandlers.has(eventType)) {
+        registeredHandlers.set(eventType, []);
+    }
+    registeredHandlers.get(eventType).push(handler);
 }
 
 /**

src/core/state.js

@@ -594,3 +594,60 @@ export function setMusicPlayerContainer($element) {
 export function setEquipmentContainer($element) {
     $equipmentContainer = $element;
 }
+
+/**
+ * Clears all cached DOM element references.
+ * Call this when the panel is destroyed or rebuilt to prevent stale jQuery references.
+ */
+export function clearDomCache() {
+    $panelContainer = null;
+    $userStatsContainer = null;
+    $infoBoxContainer = null;
+    $thoughtsContainer = null;
+    $inventoryContainer = null;
+    $questsContainer = null;
+    $musicPlayerContainer = null;
+    $equipmentContainer = null;
+}
+
+/**
+ * Clears the debug logs array.
+ * Call this on chat change to prevent memory accumulation across sessions.
+ */
+export function clearDebugLogs() {
+    debugLogs.length = 0;
+}
+
+/**
+ * Tracks jQuery delegated event handlers for cleanup.
+ * Each handler is stored with its namespace for targeted unbinding.
+ */
+const jqueryEventHandlers = [];
+
+/**
+ * Register a jQuery delegated event handler for tracking.
+ * @param {string} event - Event type with optional namespace (e.g., 'click.rpgCompanion')
+ * @param {string} selector - Delegated selector
+ * @param {Function} handler - Event handler function
+ * @returns {Function} The same handler for chaining
+ */
+export function trackJQueryHandler(event, selector, handler) {
+    jqueryEventHandlers.push({ event, selector, handler });
+    return handler;
+}
+
+/**
+ * Unbinds all tracked jQuery delegated event handlers.
+ * Call this when the extension is disabled or the panel is destroyed.
+ * @param {Function} $ - jQuery function (passed from caller)
+ */
+export function cleanupJQueryEvents($) {
+    for (const { event, selector, handler } of jqueryEventHandlers) {
+        if (handler) {
+            $(document).off(event, selector, handler);
+        } else {
+            $(document).off(event, selector);
+        }
+    }
+    jqueryEventHandlers.length = 0;
+}

src/systems/generation/injector.js

@@ -4,7 +4,8 @@
  */
 
 import { getContext } from '../../../../../../extensions.js';
-import { extension_prompt_types, extension_prompt_roles, setExtensionPrompt, eventSource, event_types } from '../../../../../../../script.js';
+import { extension_prompt_types, extension_prompt_roles, setExtensionPrompt, event_types } from '../../../../../../../script.js';
+import { on as onEvent } from '../../core/events.js';
 import {
     extensionSettings,
     committedTrackerData,
@@ -892,15 +893,16 @@ ${contextInstructionsText}
 export function initHistoryInjectionListeners() {
     // Register persistent listeners for prompt injection
     // These check pendingContextMap and only inject if there's data
+    // Use tracked registration (onEvent) so unregisterAllEvents() cleans them up
 
     // Primary: BEFORE_COMBINE for text completion (more reliable - modifies message objects)
-    eventSource.on(event_types.GENERATE_BEFORE_COMBINE_PROMPTS, onGenerateBeforeCombinePrompts);
+    onEvent(event_types.GENERATE_BEFORE_COMBINE_PROMPTS, onGenerateBeforeCombinePrompts);
 
     // Fallback: AFTER_COMBINE for text completion (string-based injection)
-    eventSource.on(event_types.GENERATE_AFTER_COMBINE_PROMPTS, onGenerateAfterCombinePrompts);
+    onEvent(event_types.GENERATE_AFTER_COMBINE_PROMPTS, onGenerateAfterCombinePrompts);
 
     // Chat completion (OpenAI, etc.)
-    eventSource.on(event_types.CHAT_COMPLETION_PROMPT_READY, onChatCompletionPromptReady);
+    onEvent(event_types.CHAT_COMPLETION_PROMPT_READY, onChatCompletionPromptReady);
 
     console.log('[RPG Companion] History injection listeners initialized');
 }

src/utils/sillyTavernExpressions.js

@@ -1,7 +1,6 @@
 import { Fuse } from '../../../../../../lib.js';
 import {
     characters,
-    eventSource,
     event_types,
     generateQuietPrompt,
     generateRaw,
@@ -11,6 +10,7 @@ import {
     substituteParamsExtended,
     this_chid
 } from '../../../../../../script.js';
+import { once as onceEvent } from '../core/events.js';
 import {
     doExtrasFetch,
     extension_settings as stExtensionSettings,
@@ -562,7 +562,7 @@ export async function classifyExpressionText(text, { characterName = '' } = {})
                     }
                 };
 
-                eventSource.once(event_types.TEXT_COMPLETION_SETTINGS_READY, onReady);
+                onceEvent(event_types.TEXT_COMPLETION_SETTINGS_READY, onReady);
 
                 const responseText = settings.promptType === PROMPT_TYPE.full
                     ? await generateQuietPrompt({ quietPrompt: prompt })