Skip to content

Conversation

@MaheshtheDev
Copy link
Member

chore: update new contributions for raycast extension

add quick add from selection with extension

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 5, 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 failed
View logs
supermemory-app b7820f9 Nov 05 2025, 11:05 PM

Copy link
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 marked this pull request as ready for review November 5, 2025 23:05
@graphite-app graphite-app bot requested a review from Dhravya November 5, 2025 23:05
@claude
Copy link

claude bot commented Nov 5, 2025

Code Review - PR #561: Raycast Extension Improvements

Overview

This PR adds significant improvements to the Raycast extension, including quick-add from selection, a new search projects command, and various code quality enhancements. Overall, this is a well-structured update with good architectural improvements.

✅ Strengths

1. Excellent Architectural Improvements

  • Higher-Order Component Pattern: The new withSupermemory.tsx HOC is a clean approach to centralize API key validation and error handling across commands
  • Modern React Patterns: Good use of usePromise and useCachedPromise from @raycast/utils for better data fetching and caching
  • Dependency Updates: Updated to latest @raycast/api (1.103.3) and @raycast/utils (2.2.1), which removes unused dependencies

2. User Experience Enhancements

  • Selection Pre-filling: The getSelectedText() integration is a great UX improvement - users can select text and immediately add/search it
  • New Projects Command: Adds valuable project management functionality with create/list capabilities
  • Better Loading States: More polished loading and empty states throughout

3. Code Quality

  • Removed unnecessary useEffect dependencies and simplified API key retrieval
  • Consistent error handling with proper toast notifications
  • Cleaner separation of concerns

⚠️ Issues & Recommendations

Critical: Incomplete Diff in PR

The diff for search-projects.tsx appears truncated in the GitHub output. The file shows 106 additions but the diff is cut off. This makes it difficult to fully review the implementation.

High Priority Issues

1. Race Condition in add-memory.tsx (Lines 28-40)

useEffect(() => {
  async function loadSelectedText() {
    try {
      const selectedText = await getSelectedText()
      if (selectedText) {
        setInitialContent(selectedText)
      }
    } catch {
      // No text selected or error getting selected text - silently fail
    }
  }
  loadSelectedText()
}, [])

Issue: The useEffect runs async operation but component might unmount before it completes.

Recommendation: Add cleanup logic:

useEffect(() => {
  let mounted = true;
  async function loadSelectedText() {
    try {
      const selectedText = await getSelectedText()
      if (selectedText && mounted) {
        setInitialContent(selectedText)
      }
    } catch {
      // Silently fail
    }
  }
  loadSelectedText()
  return () => { mounted = false }
}, [])

2. Inconsistent Error Handling in search-memories.tsx (Line 68)

const { isLoading, data: searchResults = [] } = usePromise(
  async (query: string) => {
    // ... search logic
  },
  [searchText],
)

Issue: The usePromise hook doesn't handle errors explicitly. If searchMemories() throws, users won't see proper error feedback beyond the default toast.

Recommendation: Add error handling option:

const { isLoading, data: searchResults = [], error } = usePromise(
  // ... implementation
  [searchText],
  {
    failureToastOptions: {
      title: "Search Failed",
      message: "Unable to search memories. Please try again."
    }
  }
)

3. Type Safety Issue in api.ts (Line 73)

function getApiKey(): string {
  const { apiKey } = getPreferenceValues<Preferences>();
  return apiKey;
}

Issue: The Preferences type is not defined in the visible code. This could lead to runtime errors if the type doesn't match expectations.

Recommendation: Define the type explicitly or add validation:

interface Preferences {
  apiKey: string;
}

function getApiKey(): string {
  const { apiKey } = getPreferenceValues<Preferences>();
  if (!apiKey?.trim()) {
    throw new AuthenticationError("API key is required");
  }
  return apiKey.trim();
}

Medium Priority Issues

4. Missing Description Null Check in search-projects.tsx (Line 46)

subtitle={project.description}

Issue: If project.description is undefined/null, this could cause rendering issues.

Recommendation:

subtitle={project.description || "No description"}

5. Loading State Logic in search-memories.tsx (Line 103)

const hasSearched = !isLoading && !searchResults.length

Issue: This condition is confusing - it evaluates to true when NOT loading AND no results, which seems counterintuitive to the name "hasSearched".

Recommendation: Clarify the logic or rename:

const showEmptyState = !isLoading && !searchResults.length && searchText.trim()
const showInitialState = !searchText.trim()

Low Priority / Style Issues

6. Inconsistent Formatting

The code uses tabs for indentation throughout, which is consistent within the PR but should match the repository's coding standards. Verify this aligns with your Biome configuration.

7. Silent Error Swallowing

Multiple places use empty catch blocks:

} catch {
  // No text selected or error getting selected text - silently fail
}

While appropriate for optional features, consider at least logging to console for debugging:

} catch (error) {
  console.debug("Failed to get selected text:", error);
}

🔒 Security Considerations

Good Security Practices

  • API key remains stored in preferences (not hardcoded)
  • No sensitive data logged
  • Proper authentication error handling

⚠️ Considerations

  • The API key validation happens on every command load (via HOC), which is good for security but ensure the /v3/settings endpoint is rate-limited on the backend
  • Consider caching the validation result briefly to reduce API calls

🧪 Test Coverage

Missing: No tests are included for the new functionality. Consider adding:

  • Unit tests for the withSupermemory HOC
  • Integration tests for selected text functionality
  • Tests for error states

📝 Additional Suggestions

  1. Changelog: Great improvement! The changelog now properly documents all changes with dates.

  2. Package.json: Adding contributors is excellent attribution.

  3. Dependency Cleanup: Removing unused dependencies like cross-fetch, stream-json, node-fetch is great for bundle size.

  4. Consider Adding:

    • Loading skeletons instead of empty <Detail isLoading /> in the HOC
    • Keyboard shortcuts documentation in the command descriptions
    • Analytics/telemetry for feature usage (if applicable)

Summary

Approve with minor changes recommended. This is a solid PR that significantly improves the Raycast extension. The architectural improvements with the HOC pattern and modern React hooks are excellent. The main concerns are:

  1. Fix the race condition in add-memory.tsx
  2. Add proper type definitions for Preferences
  3. Improve error handling in search functionality
  4. Add null checks for optional fields

The code follows good practices overall and aligns well with the repository's architecture. Once the high-priority issues are addressed, this will be ready to merge.


🤖 Generated with Claude Code

@claude
Copy link

claude bot commented Nov 10, 2025

Pull Request Review: Raycast Extension Updates

Overview

This PR adds new features to the Raycast extension including:

  • Quick Add from Selection functionality
  • Search Projects command
  • Updated dependencies (@raycast/api 1.103.3, @raycast/utils 2.2.1)
  • Code improvements with usePromise hook and withSupermemory HoC

Code Quality & Best Practices ✅

Strengths:

  1. Modern React Patterns: Good use of usePromise hook from @raycast/utils for data fetching
  2. Better UX: Pre-filling content from selected text is an excellent user experience improvement
  3. Code Organization: The introduction of withSupermemory HoC suggests better separation of concerns for API key validation
  4. Simplified API: Removed unnecessary async/await from getApiKey() since preferences are synchronous
  5. Changelog Updates: Good documentation of changes with clear dates and descriptions

Areas for Improvement:

  1. Missing Files: The PR references withSupermemory.tsx and search-projects.tsx but these files are not included in the diff

    • withSupermemory is imported in apps/raycast-extension/src/add-memory.tsx:13 and apps/raycast-extension/src/search-memories.tsx:13
    • search-projects is added to package.json commands but the implementation is missing
    • Action Required: Please include these files in the PR
  2. Inconsistent Formatting: The code uses tabs instead of spaces, mixing indentation styles

    • Consider standardizing with Biome formatter (per CLAUDE.md)
    • Run bun run format-lint before committing

Potential Bugs or Issues ⚠️

Critical Issues:

  1. Missing Command Implementation (apps/raycast-extension/package.json:37-41):

    {
      "name": "search-projects",
      "title": "Search Projects",
      "subtitle": "Supermemory",
      "description": "Search through your saved projects...",
      "mode": "view"
    }
    • Command is registered but search-projects.tsx file is not in the diff
    • This will cause runtime errors when users try to access the command
  2. Error Handling in getSelectedText (apps/raycast-extension/src/add-memory.tsx:33-38):

    try {
      const selectedText = await getSelectedText()
      if (selectedText) {
        setInitialContent(selectedText)
      }
    } catch {
      // No text selected or error getting selected text - silently fail
    }
    • Silent failure is acceptable for UX, but consider logging for debugging
    • Same issue in search-memories.tsx:50-55
  3. usePromise Dependency Array (apps/raycast-extension/src/search-memories.tsx:65):

    const { isLoading, data: searchResults = [] } = usePromise(
      async (query: string) => { ... },
      [searchText],
    )
    • This will trigger a new search on every keystroke after debouncing
    • Good use of throttle on List component, but verify the behavior matches expectations
  4. Removed Dependencies Without Verification:

    • Removed: cross-fetch, node-fetch, object-hash, stream-chain, stream-json
    • These were transitive dependencies of old @raycast/[email protected]
    • Verify that @raycast/[email protected] doesn't break existing functionality

Minor Issues:

  1. hasSearched Logic (apps/raycast-extension/src/search-memories.tsx:92):

    const hasSearched = !isLoading && !searchResults.length
    • This condition seems inverted - it's true when there are NO results
    • Should probably be checking if a search was performed, not if results exist
    • Current logic may cause incorrect empty state messages
  2. API Function Removed (apps/raycast-extension/src/api.ts:219):

    • Old checkApiConnection() removed, replaced with fetchSettings()
    • fetchSettings() is defined but never used in the PR
    • Consider removing if not needed, or document its purpose

Performance Considerations 🚀

Positive Changes:

  1. Reduced Bundle Size: Removing unused dependencies (cross-fetch, node-fetch, etc.) will reduce the extension size
  2. Better Caching: usePromise hook provides built-in caching and request deduplication
  3. Lazy Loading: Moving API key check to HoC prevents unnecessary API calls

Recommendations:

  1. Consider memoizing the searchMemories callback to prevent unnecessary re-renders
  2. The 50-result limit is good for performance, but consider adding pagination for large result sets

Security Concerns 🔒

Observations:

  1. API Key Handling: Good that API key validation is centralized in withSupermemory HoC
  2. No Sensitive Data in Logs: Proper error handling without exposing sensitive information
  3. URL Validation: Consider validating URLs from metadata before opening (apps/raycast-extension/src/search-memories.tsx:43)

Recommendations:

  1. Ensure withSupermemory HoC properly validates API keys before allowing component access
  2. Add URL sanitization before opening external links:
    const extractUrl = (memory: SearchResult) => {
      if (memory.metadata?.url && typeof memory.metadata.url === "string") {
        try {
          new URL(memory.metadata.url) // Validate URL format
          return memory.metadata.url
        } catch {
          return null
        }
      }
      return null
    }

Test Coverage 📋

Missing: No test files found for this extension

  • No unit tests for API functions
  • No integration tests for commands
  • Consider adding tests for:
    • API error handling
    • Selected text extraction edge cases
    • Search result parsing

Action Items

Required Before Merge:

  • Add missing withSupermemory.tsx file
  • Add missing search-projects.tsx file or remove from package.json
  • Fix hasSearched logic in search-memories.tsx:92
  • Run bun run format-lint to standardize formatting

Recommended:

  • Add URL validation in extractUrl function
  • Add basic test coverage for API functions
  • Document the purpose of fetchSettings() or remove if unused
  • Consider adding error logging for selected text failures (development mode only)

Overall Assessment

Status: ⚠️ Needs Changes

This PR introduces valuable features with improved code patterns, but has critical issues with missing files that will break functionality. The code quality improvements (usePromise, HoC pattern) are excellent, but the PR is incomplete without the referenced files.

Recommendation: Request changes to include missing files and fix the critical issues before merging.


Review generated by Claude Code - Following repository conventions from CLAUDE.md

@claude
Copy link

claude bot commented Nov 11, 2025

Code Review: Raycast Extension Updates

Summary

This PR adds valuable functionality including quick add from selection, a new search projects command, and modernizes the codebase with updated dependencies and improved patterns. Overall, this is a solid enhancement with good architectural decisions.

Positive Changes

1. Excellent UX Improvements

  • Quick add from selection: The getSelectedText() integration in both add-memory.tsx and search-memories.tsx is a great UX enhancement that reduces friction
  • New search projects command: Fills a functional gap in the extension

2. Better Code Patterns

  • withSupermemory HOC: Centralizes API key validation - excellent separation of concerns
  • usePromise hook: Modern approach replacing manual state management, reducing boilerplate
  • Simplified API: getApiKey() is now cleaner without unnecessary try-catch

3. Dependency Updates

  • Updated @raycast/api to 1.103.3 and @raycast/utils to 2.2.1
  • Removed unnecessary dependencies (cross-fetch, object-hash, stream-chain, etc.) - good cleanup

Issues & Concerns

🔴 Critical: Missing File

File: src/withSupermemory.tsx

  • Issue: This HOC is imported in both add-memory.tsx and search-memories.tsx but is not included in the PR diff
  • Impact: The extension will fail to run without this file
  • Action: Please add this file to the PR or confirm it exists

File: src/search-projects.tsx

  • Issue: Command is defined in package.json but the implementation is missing from the PR
  • Impact: Extension installation may fail or command won't work
  • Action: Please include this file in the PR

🟡 Code Quality Issues

1. Inconsistent Error Handling (search-memories.tsx:80-85)

const { isLoading, data: searchResults = [] } = usePromise(
  async (query: string) => {
    const q = query.trim()
    if (!q) return []

    const results = await searchMemories({...})
    if (!results.length) {
      await showToast({...}) // Toast shown on every search with no results
    }
    return results
  },
  [searchText],
)

Issues:

  • Toast is shown repeatedly if user is still typing (due to throttled search)
  • No error handling - errors will be swallowed silently
  • Consider showing toast only once or using a different UX pattern

Suggestion:

const { isLoading, data: searchResults = [], error } = usePromise(
  async (query: string) => {
    const q = query.trim()
    if (!q) return []
    return await searchMemories({ q, limit: 50 })
  },
  [searchText],
)

useEffect(() => {
  if (error) {
    showToast({
      style: Toast.Style.Failure,
      title: "Search Failed",
      message: error.message
    })
  }
}, [error])

2. Silent Failures (add-memory.tsx:33-40, search-memories.tsx:15-21)

try {
  const selectedText = await getSelectedText()
  if (selectedText) {
    setInitialContent(selectedText)
  }
} catch {
  // No text selected or error getting selected text - silently fail
}

Issue: All errors are silently ignored - makes debugging difficult
Suggestion: At minimum, log errors in development:

} catch (error) {
  console.debug('Could not get selected text:', error)
}

3. Race Condition Potential (add-memory.tsx:76-79)

<Form.TextArea
  id="content"
  title="Content"
  value={initialContent}
  // ...
  onChange={(value) => setInitialContent(value)}
/>

Issue: If getSelectedText() resolves after user starts typing, it will overwrite their input
Suggestion: Track if user has manually edited:

const [hasManualEdit, setHasManualEdit] = useState(false)

useEffect(() => {
  async function loadSelectedText() {
    try {
      const selectedText = await getSelectedText()
      if (selectedText && !hasManualEdit) {
        setInitialContent(selectedText)
      }
    } catch (error) {
      console.debug('Could not get selected text:', error)
    }
  }
  loadSelectedText()
}, [hasManualEdit])

4. API Breaking Change (api.ts:73-76)

function getApiKey(): string {
  const { apiKey } = getPreferenceValues<Preferences>();
  return apiKey;
}

Issue: Changed from async to sync but Preferences type is not defined in this file
Action: Add type definition or import it

5. Unused Code (api.ts:154-166)

export async function addProject(request: AddProjectRequest): Promise<Project> {
  // ... implementation
}

Issue: This function is added but never used in the PR
Action: Either use it in search-projects.tsx or remove it to avoid dead code

🟢 Minor Improvements

1. Magic Numbers

limit: 50, // Consider making this configurable

2. Accessibility (search-memories.tsx:119-123)

The subtitle tooltip is excellent for accessibility:

subtitle={{ value: truncateContent(content), tooltip: content }}

3. Code Style

  • Good use of modern React patterns (hooks, functional components)
  • Consistent use of tabs for indentation (per Biome config)
  • However, mixing single and double quotes in new files - consider consistency

Security Considerations

Good:

  • API key handling remains secure
  • No exposed secrets in the diff
  • Proper authentication checks via HOC

⚠️ Consider:

  • Add rate limiting awareness for the search throttle
  • Consider sanitizing user input before sending to API (though likely handled server-side)

Performance Considerations

Good:

  • Throttled search prevents excessive API calls
  • Using usePromise for proper loading states
  • Dependency cleanup reduces bundle size

⚠️ Consider:

  • The 500ms debounce in the original has been replaced with Raycast's throttle prop - ensure this provides similar UX
  • Consider caching search results for repeated queries

Test Coverage

Missing:

  • No tests found for this extension
  • Critical functionality like API calls, error handling, and user flows should have tests
  • Consider adding tests using Raycast's testing utilities

Action Items

Must Fix:

  1. ✅ Add withSupermemory.tsx to the PR
  2. ✅ Add search-projects.tsx to the PR
  3. ✅ Add proper error handling in usePromise usage
  4. ✅ Fix Preferences type in api.ts

Should Fix:

  1. Handle race condition in add-memory.tsx
  2. Remove or use addProject function
  3. Add basic error logging instead of completely silent failures

Nice to Have:

  1. Add unit tests
  2. Add rate limiting documentation
  3. Consider configuration for search limit

Conclusion

This is a solid enhancement to the Raycast extension with good architectural improvements. The main blockers are the missing files (withSupermemory.tsx and search-projects.tsx). Once those are added and the error handling is improved, this will be ready to merge.

Recommendation: Request changes for missing files and critical error handling, then approve after fixes.


Review performed by Claude Code following repository standards from CLAUDE.md

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