Skip to content

Conversation

@nexxeln
Copy link
Member

@nexxeln nexxeln commented Dec 3, 2025

No description provided.

@nexxeln nexxeln changed the title new for graph add docs for graph package Dec 3, 2025
@nexxeln nexxeln marked this pull request as ready for review December 3, 2025 21:01
Copy link
Member Author

nexxeln commented Dec 3, 2025


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.

@graphite-app graphite-app bot requested a review from Dhravya December 3, 2025 21:01
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 3, 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 1b9b301 Dec 04 2025, 06:58 PM

@claude
Copy link

claude bot commented Dec 3, 2025

PR Review: Memory Graph Documentation

Great work adding comprehensive documentation for the memory graph package! This is a significant improvement that will help developers integrate and use this component. Here's my detailed review:


✅ Strengths

Documentation Structure

  • Well-organized: The docs follow a logical progression (Overview → Installation → Quickstart → Examples → API Reference)
  • Clear navigation: The docs.json addition properly integrates the new section into the documentation site
  • Comprehensive coverage: All major features and use cases are documented

API Reference

  • Detailed prop documentation: Each prop has clear descriptions, types, and default values
  • Type definitions: Complete TypeScript interfaces are documented with examples
  • Exported components and hooks: Good documentation of internal exports for advanced use cases

Examples

  • Real-world scenarios: The examples cover common use cases (pagination, search, controlled state, embedding)
  • Complete code samples: Examples are production-ready with proper imports and state management
  • Multiple frameworks: Includes examples for Next.js App Router, Pages Router, and Express

🐛 Issues Found

1. Type Inconsistency in API Reference

Location: apps/docs/memory-graph/api-reference.mdx:36-39

The documented MemoryEntry interface includes a memoryRelations field, but this conflicts with the actual implementation:

// Documented (api-reference.mdx)
memoryRelations?: Array<{
  relationType: 'updates' | 'extends' | 'derives';
  targetMemoryId: string;
}> | null;

However, in packages/memory-graph/src/api-types.ts:22-27, the actual implementation shows these fields are separate:

// Actual implementation
updatesMemoryId?: string | null
nextVersionId?: string | null
relation?: "updates" | "extends" | "derives" | null

Recommendation: Update the API reference to match the actual MemoryEntry type from api-types.ts, including all fields like updatesMemoryId, nextVersionId, sourceAddedAt, sourceRelevanceScore, etc.


2. Security: API Key Exposure Warning Could Be Stronger

Location: apps/docs/memory-graph/quickstart.mdx:127-129

Current warning:

<Warning>
  Never expose your Supermemory API key to the client. Always fetch data through your backend.
</Warning>

Recommendation: Consider adding specific guidance:

<Warning>
  **Never expose your Supermemory API key to the client.** Always fetch data through your backend.
  
  ⚠️ Do NOT:
  - Commit API keys to version control
  - Use API keys in client-side code (React components, browser JavaScript)
  - Include API keys in public repositories
  
  ✅ DO:
  - Store keys in environment variables
  - Use server-side API routes or backend proxies
  - Add `.env*` to your `.gitignore`
</Warning>

3. README Simplification May Have Removed Useful Information

Location: packages/memory-graph/README.md

The PR significantly simplifies the README (from 378 lines to 94 lines). While this improves readability, some useful information was removed:

Removed content that might be valuable:

  • Feature highlights (WebGL-powered rendering, interactive exploration, etc.)
  • Backend proxy examples
  • Detailed prop explanations
  • Advanced usage patterns

Recommendation:

  • Keep the simplified README but add a link to the full documentation at the top
  • Consider keeping a brief "Features" section in the README

Example addition:

## Documentation

📚 **[View Full Documentation](https://docs.supermemory.ai/memory-graph/overview)**

## Features

- 🎨 Canvas-based rendering for smooth performance
- 🔍 Interactive pan, zoom, and drag
- 🌐 Space filtering and search
- 📱 Responsive design (mobile & desktop)
- 📦 Zero configuration with automatic CSS injection

4. Minor: Missing NPM Badge Link

Location: packages/memory-graph/README.md:5

The NPM version badge links to the package, but there's no direct "View on NPM" link.

Recommendation: Add after the badges:

[View on NPM](https://www.npmjs.com/package/@supermemory/memory-graph) | [Documentation](https://docs.supermemory.ai/memory-graph/overview)

5. Code Example: Potential Issue with State Management

Location: apps/docs/memory-graph/examples.mdx:406

In the mobile-responsive example:

useEffect(() => {
  const checkMobile = () => {
    setIsMobile(window.innerWidth < 768);
  };

  checkMobile();
  window.addEventListener('resize', checkMobile);
  return () => window.removeEventListener('resize', checkMobile);
}, []);

Recommendation: This will cause the entire graph to re-render when switching between mobile/desktop, losing zoom/pan state. Consider adding a note about this behavior or suggesting useMemo for the variant calculation if state preservation is important.


💡 Suggestions for Improvement

1. Add Troubleshooting Section

Consider adding a troubleshooting guide for common issues:

  • "Graph is not rendering" → Check container height
  • "Styles are not loading" → Import order issues
  • "Performance issues" → Enable pagination, reduce memory limit

2. Performance Best Practices

Add a dedicated section on performance optimization:

  • When to use pagination
  • Optimal memoryLimit values
  • Browser compatibility notes

3. TypeScript Integration Guide

Add examples showing:

  • How to extend types
  • Custom theming with TypeScript
  • Type-safe wrapper components

4. Accessibility Considerations

The graph is canvas-based which can present accessibility challenges. Consider documenting:

  • Keyboard navigation support (if any)
  • Screen reader compatibility
  • ARIA label recommendations for the container

🔒 Security Review

✅ Good Security Practices

  • API key handling is correctly documented to be server-side only
  • Examples show proper environment variable usage
  • No client-side API key exposure in examples

⚠️ Recommendations

  1. CSP Considerations: Document any Content Security Policy requirements for Canvas 2D API
  2. Input Validation: Mention that document data should be validated/sanitized before rendering
  3. XSS Prevention: If document titles/content come from user input, note they should be sanitized

📊 Test Coverage

This PR is documentation-only, so test coverage changes aren't applicable. However, for future work, consider:

  • Adding tests that validate documentation examples compile correctly
  • Type-checking all code snippets in the documentation

✅ Final Verdict

Overall: Excellent PR

This documentation significantly improves the developer experience for the memory-graph package. The issues found are minor and mostly related to ensuring documentation perfectly matches the implementation.

Recommended Actions Before Merge:

  1. Must fix: Update API reference type definitions to match actual implementation
  2. 🔧 Should fix: Enhance security warning in quickstart guide
  3. 💭 Consider: Add link to full docs in README
  4. 💭 Consider: Add troubleshooting section

Approval Status

I recommend approving with minor changes. The type inconsistency (#1) should be fixed before merging, but other suggestions can be addressed in follow-up PRs.

Great work @nexxeln! 🎉

@claude
Copy link

claude bot commented Dec 3, 2025

PR Review: Memory Graph Documentation

This is an excellent documentation PR! The docs are comprehensive, well-structured, and provide great value to users. Here's my detailed feedback:


✅ Strengths

Documentation Structure

  • Clear organization: Overview → Installation → Quick Start → API Reference → Examples flows logically
  • Progressive disclosure: Simple examples first, complex patterns later
  • Good navigation: Proper integration with docs.json sidebar

Content Quality

  • Comprehensive API reference: All props documented with types, defaults, and descriptions (apps/docs/memory-graph/api-reference.mdx)
  • Practical examples: 8+ real-world use cases covering pagination, search, controlled state, mobile, etc. (apps/docs/memory-graph/examples.mdx)
  • Type safety: Complete TypeScript types exported and documented

Code Examples

  • Functional and realistic: Examples use proper React patterns (hooks, effects, state management)
  • Multiple framework support: Next.js App Router, Pages Router, Express examples
  • Good practices: Proper error handling, loading states, and user feedback

🔍 Issues & Recommendations

1. Security - API Key Exposure Warning ⚠️

Location: apps/docs/memory-graph/quickstart.mdx:127-129

Current:

<Warning>
  Never expose your Supermemory API key to the client. Always fetch data through your backend.
</Warning>

Issue: While the warning is present, some examples show direct API calls without explicitly demonstrating the proxy pattern.

Recommendation: Add a callout in the Quick Start showing the architecture:

Client (React) → Your Backend API (/api/graph) → Supermemory API
                ✅ API Key here        ❌ Never here

2. Type Accuracy - API Types Documentation

Location: apps/docs/memory-graph/api-reference.mdx:100-176

Issue: The documented types match the source (packages/memory-graph/src/api-types.ts), which is good! However, some fields could use more context:

  • processingMetadata?: Record<string, unknown> | null - What does this contain?
  • raw?: string | null - Raw content before processing?
  • summaryEmbedding?: number[] - Dimensionality? (e.g., typically 768 or 1536 dimensions)

Recommendation: Add brief inline comments in the API reference explaining these fields.


3. Performance Guidance

Location: apps/docs/memory-graph/overview.mdx:22-29

Excellent coverage of:

  • Canvas rendering strategy
  • Viewport culling
  • LOD optimization

Missing:

  • Recommended limit values for initial load (you show 500 in examples)
  • Memory consumption estimates (e.g., "~500 documents = ~X MB")
  • When to use pagination vs full load

Recommendation: Add a performance section to quickstart.mdx:

## Performance Tips

- **Initial load**: Start with 100-200 documents for instant rendering
- **Pagination**: Use `limit: 100` per page for large datasets (1000+ docs)
- **Memory usage**: Each document with 10 memories ≈ 50KB in memory

4. Error Handling in Examples

Location: apps/docs/memory-graph/examples.mdx:67-78 (pagination example)

Current:

const res = await fetch(`/api/graph?page=${pageNum}&limit=100`);
const data = await res.json();

Issue: No error handling in the pagination example (though it exists in other examples).

Recommendation: Add try-catch to show best practices:

try {
  const res = await fetch(`/api/graph?page=${pageNum}&limit=100`);
  if (!res.ok) throw new Error(`HTTP ${res.status}`);
  const data = await res.json();
  // ... rest
} catch (error) {
  console.error('Failed to load documents:', error);
  setError(error as Error);
}

5. README Truncation

Location: packages/memory-graph/README.md

Issue: The diff shows the README was truncated at line 94 with "[370 lines truncated]". Need to verify the full content is correctly updated.

Action Required: Please confirm the README.md changes are complete and aligned with the new docs.


6. Minor: CSS Import Clarity

Location: apps/docs/memory-graph/quickstart.mdx:148-166

Current explanation is good, but could clarify:

Add:

### When to use explicit CSS import:
- Custom build tools (Webpack, Rollup) with specific CSS handling
- Server-side rendering with manual style injection
- Testing environments that don't auto-inject styles

### When automatic injection works:
- Next.js (App Router or Pages Router)
- Create React App
- Vite projects
- Most modern React setups

📊 Test Coverage

No test-related changes were included in this PR. While documentation PRs don't require tests, consider:

  1. Example validation: Run the code examples to ensure they actually work
  2. Link checking: Verify all internal links (e.g., /memory-graph/examples) resolve correctly
  3. Type checking: Ensure TypeScript types in docs match the actual exports

Suggested command:

cd apps/docs && bun run build  # Verify docs build without errors

🎯 Overall Assessment

Category Rating Notes
Documentation Quality ⭐⭐⭐⭐⭐ Comprehensive and well-written
Code Examples ⭐⭐⭐⭐ Functional but minor error handling gaps
Type Safety ⭐⭐⭐⭐⭐ Excellent TypeScript documentation
Security ⭐⭐⭐⭐ Warning present but could be more prominent
Performance Guidance ⭐⭐⭐⭐ Good coverage, could add more specifics

✅ Approval Recommendation

LGTM with minor suggestions. This is high-quality documentation that significantly improves the developer experience. The issues raised are minor and can be addressed in a follow-up PR or before merge.

Before Merging:

  1. ✅ Verify README.md is fully updated (not truncated)
  2. ✅ Run bun run build in apps/docs to confirm no errors
  3. 🔧 Consider: Add security architecture diagram to Quick Start
  4. 🔧 Consider: Enhance error handling in pagination example

Great work! 🚀

@graphite-app
Copy link

graphite-app bot commented Dec 4, 2025

Merge activity

@graphite-app
Copy link

graphite-app bot commented Dec 4, 2025

Illustrated gif. A hand appears and holds up an oversized thumb, giving us a thumbs up. (Added via Giphy)

@graphite-app graphite-app bot merged commit 1b9b301 into main Dec 4, 2025
4 of 6 checks passed
@claude
Copy link

claude bot commented Dec 4, 2025

Code Review - Memory Graph Documentation

Summary

This PR adds comprehensive documentation for the @supermemory/memory-graph package. Overall, this is excellent work with well-structured, clear, and detailed documentation. The docs are developer-friendly and provide great practical examples.


Strengths

Documentation Quality:

  • Comprehensive coverage: API reference, examples, quickstart, and overview all work together cohesively
  • Clear organization: Logical flow from overview to installation to quickstart to examples to API reference
  • Practical examples: Real-world use cases including pagination, search, controlled state, and RSC patterns
  • Good TypeScript support: Type definitions are well-documented with proper interfaces

Code Examples:

  • Examples are production-ready and follow React best practices
  • Good use of both controlled and uncontrolled patterns
  • Includes both App Router and Pages Router examples for Next.js
  • Mobile-responsive example shows thoughtful consideration

README Improvements:

  • Simplified from 378 lines to 94 lines - much more focused
  • Clear external link to full documentation
  • Removes redundant content that now lives in proper docs

Suggestions for Improvement

  1. Security Concerns (Important)
  • Issue: API key handling in examples could be clearer about security best practices
  • Recommendation: Add a prominent Warning callout in the quickstart.mdx Backend API Route section emphasizing that API keys should never be exposed to the client
  1. Type Consistency
  • Issue: In api-reference.mdx, the MemoryEntry interface shows both new and legacy fields, which might confuse users
  • Recommendation: Add deprecation notes to legacy fields explaining which fields to use instead and when they'll be removed
  1. Error Handling in Examples
  • Issue: The pagination example in examples.mdx does not show error handling for fetch calls
  • Recommendation: Add try-catch blocks to async functions for better production patterns
  1. Performance Guidance
  • Issue: The pagination example uses 100 items per page without explaining the reasoning
  • Recommendation: Add a Note explaining optimal pagination sizes (50-100 documents per page)
  1. Mobile-Responsive Example Enhancement
  • Issue: The mobile example does not account for SSR properly with useState(false)
  • Recommendation: Initialize state with a function that checks typeof window !== 'undefined'

Nits (Very Minor)

  1. Consistent terminology: Use either Space or Spaces consistently throughout docs
  2. README link: The README says View full documentation but does not include the actual URL
  3. API Reference: Consider clarifying which types are public API vs internal implementation details

Overall Assessment

Quality: 5/5 stars
Completeness: 4.5/5 stars
Developer Experience: 5/5 stars

This is high-quality documentation that will significantly improve the developer experience. The suggestions above are mostly about adding extra safety guardrails and minor enhancements. The core content is excellent.

Recommendation: Approve with minor suggestions

The security callout about API keys is the most important addition. Everything else is optional polish.

Great work!

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.

3 participants