Skip to content

feat (extension) : Auto Search Toggle for Chat Applications#418

Merged
graphite-app[bot] merged 1 commit into
mainfrom
09-09-feat_auto_search_for_memories_and_list_view
Sep 10, 2025
Merged

feat (extension) : Auto Search Toggle for Chat Applications#418
graphite-app[bot] merged 1 commit into
mainfrom
09-09-feat_auto_search_for_memories_and_list_view

Conversation

@MaheshtheDev
Copy link
Copy Markdown
Member

@MaheshtheDev MaheshtheDev commented Sep 10, 2025

Adds user-controlled auto search functionality for memories across ChatGPT, Claude, and T3.chat with a settings toggle in the extension popup.

Changes

  • Settings UI: Added new "Settings" tab in popup with toggle to enable/disable auto search
  • Auto Search: Automatically searches user memories while typing in chat apps (disabled by default)
  • Chat Integration: Supports ChatGPT, Claude, and T3.chat with consistent behavior
  • User Control: Users can enable/disable auto search via Settings tab
  • Storage: Added AUTO_SEARCH_ENABLED storage key with default value false

Features

  • ✅ Auto search memories while typing (when enabled)
  • ✅ Manual search always available via supermemory icons
  • ✅ Works across all supported chat platforms
  • ✅ Real-time toggle without requiring page refresh

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Sep 10, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
supermemory-app d7cc7d8 Sep 10 2025, 04:03 AM

Copy link
Copy Markdown
Member Author


How to use the Graphite Merge Queue

Add the label Main to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MaheshtheDev MaheshtheDev changed the title feat: auto search for memories and list view feat: auto search for memories Sep 10, 2025
@MaheshtheDev MaheshtheDev changed the title feat: auto search for memories feat: Auto Search Toggle for Chat Applications Sep 10, 2025
@MaheshtheDev MaheshtheDev changed the title feat: Auto Search Toggle for Chat Applications feat (extension) : Auto Search Toggle for Chat Applications Sep 10, 2025
@MaheshtheDev MaheshtheDev marked this pull request as ready for review September 10, 2025 03:12
@graphite-app graphite-app Bot requested a review from Dhravya September 10, 2025 03:12
@claude
Copy link
Copy Markdown

claude Bot commented Sep 10, 2025

Pull Request Review: Auto Search Toggle for Chat Applications

Overview

This PR refactors the browser extension's content script architecture and adds auto-search functionality for chat applications. The changes are substantial, involving a complete restructuring from a single content script to modular, platform-specific implementations.

Code Quality & Best Practices

✅ Positive Aspects

  • Excellent modularization: Breaking down the monolithic content.ts into platform-specific files (chatgpt.ts, claude.ts, t3.ts, twitter.ts) significantly improves maintainability
  • Consistent error handling: Good use of try-catch blocks throughout the codebase
  • Resource cleanup: Proper cleanup of timers and observers to prevent memory leaks
  • Debouncing implementation: Smart use of debouncing for auto-search to avoid excessive API calls

⚠️ Areas for Improvement

Code Duplication

There's significant code duplication across platform files, especially in:

  • Memory fetching logic (getRelatedMemoriesForChatGPT, getRelatedMemoriesForClaude, getRelatedMemoriesForT3)
  • DOM manipulation patterns
  • Error handling structures

Recommendation: Extract common functionality into shared.ts or create base classes/higher-order functions.

Inconsistent Naming

  • setupChatGPTAutoFetch() vs setupClaudeAutoSearch() vs setupT3AutoSearch()
  • Should standardize on either "AutoFetch" or "AutoSearch"

Memory Leaks Risk

In several files, observers and intervals are created but cleanup could be more robust:

// Example from chatgpt.ts - cleanup could be consolidated
if (chatGPTRouteObserver) {
    chatGPTRouteObserver.disconnect()
}
if (chatGPTUrlCheckInterval) {
    clearInterval(chatGPTUrlCheckInterval)
}

Performance Considerations

✅ Good Practices

  • Throttling and debouncing: Prevents excessive API calls during rapid typing
  • Conditional loading: Platform-specific code only loads when needed
  • Timeout handling: API requests have reasonable timeouts (10 seconds)

⚠️ Performance Concerns

  • DOM Polling: Heavy use of setInterval for DOM changes detection (every 1-2 seconds)
  • Multiple observers: Each platform runs its own mutation observers, which could be resource-intensive
  • Memory usage: Large strings being stored in DOM dataset attributes could impact memory

Recommendations:

  1. Consider using IntersectionObserver where appropriate instead of polling
  2. Implement a single observer with platform-specific handlers
  3. Store memory data in extension storage rather than DOM attributes

Security Concerns

⚠️ Moderate Security Issues

Content Injection

The code directly injects HTML content into input fields:

// claude.ts:169
textareaElement.innerHTML = `${currentContent}<br>Supermemories: ${response.data}`

Risk: If response.data contains malicious HTML/JavaScript, it could lead to XSS attacks.

Recommendation: Use textContent instead of innerHTML, or properly sanitize HTML content.

Sensitive Data Exposure

// shared.ts:26
console.log("=== PROMPT CAPTURED ===")
console.log(messageData)

Risk: User prompts might contain sensitive information being logged to console.

Recommendation: Remove or add conditional logging for production builds.

Functionality & Logic Issues

✅ Solid Implementation

  • Settings persistence: Good use of chrome storage for user preferences
  • Route change detection: Robust handling of SPA route changes
  • Error boundaries: Proper error handling prevents crashes

⚠️ Potential Bugs

Race Conditions

Multiple async operations without proper sequencing could cause issues:

// Background script - no await on trackEvent
await trackEvent(eventSource)
sendResponse({ success: true, data: memories })

DOM Element Detection

Fragile DOM selectors that could break with UI updates:

// chatgpt.ts:465 - very specific class-based selector
const targetContainers = document.querySelectorAll(".relative.flex-1.flex.items-center.gap-2.shrink.min-w-0")

Recommendation: Use more robust selectors or add fallbacks.

Test Coverage

❌ Missing Test Coverage

The PR lacks any test files or test updates. Given the complexity of the changes, this is concerning.

Critical Missing Tests:

  • Unit tests for debouncing logic
  • Integration tests for platform-specific functionality
  • Security tests for content injection prevention
  • Performance tests for memory usage

Recommendation: Add comprehensive test coverage before merging, especially for:

  1. Auto-search functionality
  2. Settings toggle behavior
  3. Platform detection and routing
  4. Error handling scenarios

Architecture & Design

✅ Good Architecture Decisions

  • Separation of concerns: Clear separation between platforms
  • Event-driven design: Good use of message passing between content and background scripts
  • Configuration management: Centralized constants and UI configuration

⚠️ Design Concerns

  • Tight coupling: Platform implementations are tightly coupled to specific DOM structures
  • Monolithic functions: Some functions (like initializeChatGPT) are doing too many things

Minor Issues

  1. Typo in commit message: "togglee" should be "toggle"
  2. Inconsistent formatting: Mix of arrow functions and regular functions
  3. Magic numbers: Timeouts and intervals use magic numbers (should be constants)
  4. Debug logging: Several console.log statements should be removed for production

Overall Assessment

This is a substantial improvement to the extension's architecture with good separation of concerns. However, the PR has some security and performance concerns that should be addressed before merging.

Recommendation: Approve with Changes

Must-fix before merge:

  1. Address XSS vulnerability in HTML injection
  2. Remove sensitive data logging
  3. Add basic test coverage
  4. Extract common functionality to reduce duplication

Nice-to-have improvements:

  1. Performance optimizations (reduce DOM polling)
  2. More robust DOM selectors
  3. Consolidated cleanup methods

The auto-search feature is well-implemented with proper user controls and storage management. The modular architecture will make future maintenance much easier.

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown

claude Bot commented Sep 10, 2025

Code Review: Auto Search Toggle for Chat Applications

Summary

This PR adds a user-controlled auto search functionality for memories across ChatGPT, Claude, and T3.chat with a settings toggle. The implementation refactors the monolithic content script into platform-specific modules, which is a positive architectural change.

Code Quality & Best Practices ✅

Strengths:

  • Good separation of concerns: Breaking the single large content.ts file (674 lines) into platform-specific modules improves maintainability
  • Consistent naming conventions: Following the existing codebase patterns with ELEMENT_IDS, MESSAGE_TYPES, etc.
  • Proper TypeScript usage: Good type definitions and consistent with existing codebase patterns
  • Event handling: Proper cleanup of observers and timeouts to prevent memory leaks

Areas for improvement:

  • Code duplication: Similar DOM manipulation and memory fetching logic across chatgpt.ts, claude.ts, and t3.ts files. Consider extracting common utilities.
  • Magic numbers: Several hardcoded timeouts (2000ms, 1000ms, 300ms) could be moved to UI_CONFIG constants for consistency

Potential Bugs & Issues ⚠️

  1. Race conditions in observer setup: Multiple timeout-based initializations could conflict if pages load/navigate quickly
  2. Memory leak potential: Observer cleanup logic exists but could be improved with more defensive programming
  3. DOM selector fragility: Heavy reliance on specific CSS selectors (e.g., .relative.flex-1.flex.items-center.gap-2.shrink.min-w-0) that could break with platform updates
  4. Error handling: Limited error boundaries around DOM operations that could fail silently

Performance Considerations 🔄

Positive aspects:

  • Debouncing: Good implementation of debounced auto-search functionality
  • Throttling: Observer callbacks are throttled to prevent excessive DOM operations

Concerns:

  • Multiple observers per platform: Each platform creates multiple mutation observers that could impact performance on content-heavy pages
  • Frequent DOM queries: Some functions repeatedly query the DOM instead of caching elements
  • Auto-search overhead: When enabled, auto-search will trigger on every input change, potentially causing API rate limiting

Security Considerations 🔒

Generally secure implementation:

  • Content scripts properly sandboxed
  • No direct eval() or innerHTML injections with user data
  • Proper message passing between extension components

Minor concerns:

  • XSS prevention: When injecting memories into chat inputs, ensure proper sanitization (current implementation appears safe but worth monitoring)

Testing & Missing Coverage 🧪

The PR lacks:

  • Unit tests for the new auto-search functionality
  • Integration tests for platform-specific implementations
  • Error handling test cases

Architecture & Maintainability 📐

Excellent refactoring:

  • Breaking down the monolithic content script improves code organization
  • Clear module boundaries make platform-specific logic easier to maintain
  • Consistent API across different chat platforms

Recommendations

  1. Extract shared utilities: Create a shared utility module for common DOM operations and memory fetching logic
  2. Add comprehensive error handling: Wrap DOM operations in try-catch blocks with proper fallbacks
  3. Implement feature flags: Consider making auto-search configurable per platform
  4. Add performance monitoring: Track auto-search frequency and API response times
  5. Create integration tests: Test the functionality across different chat platforms
  6. Document breaking changes: The refactoring might affect other extensions or integrations

Conclusion

This is a solid implementation that significantly improves the codebase architecture while adding valuable functionality. The auto-search feature is well-implemented with proper user controls. The main areas for improvement are around code deduplication, error handling, and test coverage.

Overall Rating: ✅ Approve with suggestions

The benefits of the architectural improvements and new functionality outweigh the minor concerns raised above.

🤖 Generated with Claude Code

@Dhravya Dhravya force-pushed the 09-09-feat_auto_search_for_memories_and_list_view branch from c766167 to 1ed04e8 Compare September 10, 2025 03:55
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Sep 10, 2025

Merge activity

@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Sep 10, 2025

SpongeBob gif. SpongeBob pretends to crank his fist like a jack-in-the-box, and his thumb rises and pops out for a thumbs up. He then gestures to his thumb like 'eh? What do you think?' (Added via Giphy)

Adds user-controlled auto search functionality for memories across ChatGPT, Claude, and T3.chat with a settings toggle in the extension popup.

  Changes

  - Settings UI: Added new "Settings" tab in popup with toggle to enable/disable auto search
  - Auto Search: Automatically searches user memories while typing in chat apps (disabled by default)
  - Chat Integration: Supports ChatGPT, Claude, and T3.chat with consistent behavior
  - User Control: Users can enable/disable auto search via Settings tab
  - Storage: Added AUTO_SEARCH_ENABLED storage key with default value false

  Features

  - ✅ Auto search memories while typing (when enabled)
  - ✅ Manual search always available via supermemory icons
  - ✅ Works across all supported chat platforms
  - ✅ Real-time toggle without requiring page refresh
@graphite-app graphite-app Bot force-pushed the 09-09-feat_auto_search_for_memories_and_list_view branch from 1ed04e8 to d7cc7d8 Compare September 10, 2025 03:55
@claude
Copy link
Copy Markdown

claude Bot commented Sep 10, 2025

Pull Request Review

🎯 Overall Assessment

This PR successfully refactors the browser extension content script architecture and adds auto-search functionality. The code is generally well-structured and follows good patterns.

✅ Strengths

Architecture Improvements:

  • Excellent refactoring from monolithic content.ts to platform-specific modules (chatgpt.ts, claude.ts, t3.ts)
  • Clean separation of concerns with shared utilities in shared.ts
  • Consistent route detection and observer patterns across platforms
  • Good use of debouncing and throttling for performance optimization

Feature Implementation:

  • Well-implemented auto-search toggle with persistent storage
  • Proper user consent (disabled by default) for privacy compliance
  • Comprehensive platform support (ChatGPT, Claude, T3.chat)
  • Consistent UI feedback across platforms

⚠️ Issues to Address

Performance & Memory:

// chatgpt.ts:48 - Missing cleanup
function setupChatGPTRouteChangeDetection() {
    if (chatGPTRouteObserver) {
        chatGPTRouteObserver.disconnect()  // ✅ Good
    }
    // But missing cleanup in some error paths
}

Security:

  • Line 22 in shared.ts: Typo "enxtension" in console.log could indicate copy-paste error
  • Background script should validate data.content before processing (line 118-121 in background.ts)

Error Handling:

// Multiple files - Generic error handling
} catch (error) {
    console.error("Error:", error)
    // Should include more context and user-friendly fallbacks
}

Code Quality:

  • Several magic numbers that should be constants (timeouts, delays)
  • Inconsistent error message formatting across platforms
  • Some complex nested DOM queries could be simplified

🔧 Recommended Fixes

  1. Add input validation in background script:
const payload: MemoryPayload = {
    containerTags: [containerTag],
    content: data.content ? 
        String(data.content).slice(0, 10000) : // Limit size
        `${data.highlightedText}\n\n${data.html}\n\n${data?.url}`,
    metadata: { sm_source: "consumer" },
}
  1. Fix typo in shared.ts:22:
console.log("Response from extension:", response)  // Fix "enxtension"
  1. Add cleanup for all observers:
// Ensure cleanup in finally blocks or error paths
window.addEventListener('beforeunload', () => {
    if (chatGPTObserver) chatGPTObserver.disconnect()
    // ... cleanup other resources
})

🧪 Testing Considerations

  • Cross-browser compatibility: Test in Chrome, Firefox, Edge
  • Permission handling: Verify storage permissions work correctly
  • Rate limiting: Test API timeout scenarios
  • Memory leaks: Test observer cleanup on tab navigation
  • Auto-search edge cases: Test with very long text, special characters

🔒 Security Assessment

  • ✅ Proper domain validation for token storage
  • ✅ User consent for auto-search feature
  • ⚠️ Should validate/sanitize content size and format
  • ✅ Appropriate use of storage APIs

📈 Performance Notes

  • Good use of debouncing (1.5s delay)
  • Proper throttling of observers (300ms)
  • Consider adding request cancellation for rapid typing
  • Memory usage looks reasonable with proper cleanup

🎯 Suggested Improvements

  1. Add JSDoc comments for public functions
  2. Consider using TypeScript strict mode
  3. Add unit tests for core utilities
  4. Implement exponential backoff for API failures
  5. Add telemetry for feature usage analytics

Final Verdict

Approval with minor fixes recommended

This is a solid improvement to the codebase. The refactoring enhances maintainability and the new auto-search feature is well-implemented. Address the security validation and typo, and this will be ready to merge.

Great work on the comprehensive platform support and user experience improvements!

@graphite-app graphite-app Bot merged commit d7cc7d8 into main Sep 10, 2025
2 of 3 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Sep 10, 2025

Pull Request Review: Auto Search Toggle for Chat Applications

I've completed a comprehensive review of PR #418. Overall, this is a well-structured implementation that adds valuable functionality, but there are several areas for improvement.

Strengths

  • Clean Architecture: Good separation of concerns by splitting content scripts by platform (chatgpt.ts, claude.ts, t3.ts)
  • User Experience: The toggle in settings is intuitive and provides clear control over the auto-search feature
  • Consistent Implementation: Auto-search behavior is consistent across all three chat platforms
  • Performance Optimization: Proper use of debouncing (1500ms) and throttling (300ms) to prevent excessive API calls

🔧 Code Quality & Best Practices

Positive:

  • Good use of TypeScript throughout
  • Consistent error handling patterns
  • Proper cleanup of observers and intervals

Areas for Improvement:

  • Some functions are quite large (e.g., setupChatGPTRouteChangeDetection at ~60 lines) - consider breaking into smaller functions
  • Minor text inconsistency: "Save to supermemory" vs "sync to supermemory" in background.ts:26

🐛 Potential Issues

  1. Memory Leak Risk: Multiple observers and intervals are created but cleanup could be more robust

    • chatGPTRouteObserver, chatGPTUrlCheckInterval, and chatGPTObserverThrottle should have comprehensive cleanup in all error scenarios
  2. Race Conditions: Auto-fetch setup functions could potentially run multiple times simultaneously

    • Consider adding locks or flags to prevent duplicate setup
  3. DOM Query Fragility: Extensive use of CSS selectors that could break with platform UI updates

    • Example: .relative.flex-1.flex.items-center.gap-2.shrink.min-w-0 is quite specific

📊 Performance Considerations

Good:

  • Debouncing prevents API spam
  • Observer throttling reduces CPU usage
  • Route change detection is optimized

Concerns:

  • Multiple MutationObservers running simultaneously could impact performance
  • Consider using requestIdleCallback for non-critical DOM operations

🔒 Security

  • No major security issues identified
  • Proper message validation between content scripts and background
  • Safe storage of user preferences

🧪 Test Coverage

Critical Issue: No test files found in the browser extension directory. For a feature that:

  • Manipulates DOM across multiple platforms
  • Has complex auto-search logic
  • Includes user preference storage
  • Handles API communications

Recommendation: Add tests for:

  • Auto-search toggle functionality
  • Platform-specific DOM manipulation
  • API timeout handling
  • Storage operations

💡 Suggestions for Improvement

  1. Add Error Boundaries: Wrap auto-search operations in try-catch blocks to prevent crashes
  2. Improve Logging: Add more descriptive console messages for debugging
  3. Configuration: Make debounce delays configurable via settings
  4. Accessibility: Add ARIA labels to the settings toggle
  5. Documentation: Add JSDoc comments for complex functions

🎯 Priority Fixes

  1. High: Add test coverage
  2. Medium: Improve observer cleanup
  3. Medium: Add error boundaries around DOM operations
  4. Low: Break down large functions

The implementation demonstrates solid understanding of browser extension architecture and provides genuine value to users. With the suggested improvements, particularly around testing and error handling, this would be production-ready.

Great work on the feature implementation! 🚀

@MaheshtheDev MaheshtheDev deleted the 09-09-feat_auto_search_for_memories_and_list_view branch October 29, 2025 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants