Skip to content

fix ui issues and package issue#610

Merged
Dhravya merged 2 commits into
mainfrom
mahesh/fixes-ui
Dec 7, 2025
Merged

fix ui issues and package issue#610
Dhravya merged 2 commits into
mainfrom
mahesh/fixes-ui

Conversation

@MaheshtheDev
Copy link
Copy Markdown
Member

No description provided.

@MaheshtheDev MaheshtheDev marked this pull request as ready for review December 7, 2025 01:21
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Dec 7, 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 b5c81a2 Dec 07 2025, 01:26 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.

@Dhravya Dhravya merged commit 41d24a4 into main Dec 7, 2025
2 of 3 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Dec 7, 2025

Code Review: PR #610 - Fix UI Issues and Package Issue

Thank you for working on consolidating the memory-graph package! I've conducted a thorough review of the changes. Here's my feedback:

✅ What's Working Well

1. Correct Import Path Migration

All 8 files in apps/web/ have been properly updated:

  • ✅ Import paths changed from @repo/ui/colors@repo/ui/memory-graph/constants
  • ✅ Import paths changed from @supermemory/memory-graph@repo/ui/memory-graph
  • ✅ The target exports exist and are properly structured in packages/ui/memory-graph/constants.ts

Files correctly updated:

  • components/graph-dialog.tsx
  • components/memory-list-view.tsx
  • components/masonry-memory-list.tsx
  • components/content-cards/{google-docs,note,tweet,website}.tsx
  • lib/document-icon.tsx

2. Package Consolidation Strategy

The move from @supermemory/memory-graph to @repo/ui/memory-graph makes sense:

  • Reduces duplication
  • Better monorepo integration
  • Simplified dependency management

3. Dependency Addition

Adding react-markdown to apps/web/package.json is fine (v10.1.0 is current).


⚠️ Critical Issues Found

1. Incomplete Migration - memory-graph-playground App Not Updated

The apps/memory-graph-playground/ app is still using the old package:

File: apps/memory-graph-playground/package.json

"dependencies": {
  "@supermemory/memory-graph": "workspace:*",  // ❌ Still using old package
  ...
}

File: apps/memory-graph-playground/src/app/page.tsx

import {
  MemoryGraph,
  type DocumentWithMemories,
} from "@supermemory/memory-graph"  // ❌ Still using old import

Impact: This app will break when/if the old package is removed. All apps should be updated consistently.

2. Documentation Not Updated

Found 5 documentation files in apps/docs/memory-graph/ that still reference the old package:

  • installation.mdx
  • npm.mdx
  • quickstart.mdx
  • api-reference.mdx
  • examples.mdx

If this package is being deprecated for internal use, the docs should be updated accordingly.


🔍 Other Observations

Code Quality

  • ✅ No syntax errors or type issues introduced
  • ✅ Consistent import style across all updated files
  • ✅ No security vulnerabilities introduced

Performance

  • ✅ No performance concerns - these are just import path changes
  • ✅ Bundle size should remain similar or improve slightly

Test Coverage

  • ⚠️ No tests were added or modified in this PR
  • ℹ️ Consider testing that imports resolve correctly in a build

bun.lock Changes

  • ✅ Lock file updates look normal (dependency version bumps)
  • ✅ Added apps/raycast-extension workspace (seems intentional)
  • ⚠️ configVersion field was removed from lock file - verify this is intentional

📋 Recommendations

Required Changes

  1. Update memory-graph-playground app:

    # In apps/memory-graph-playground/package.json
    # Remove: "@supermemory/memory-graph": "workspace:*"
    # Add: Dependency on @repo/ui or configure path alias
    
    # In apps/memory-graph-playground/src/app/page.tsx
    # Change: from "@supermemory/memory-graph"
    # To: from "@repo/ui/memory-graph"
  2. Update or clarify documentation:

    • Update docs if the package is being deprecated for internal use
    • OR clarify if @supermemory/memory-graph will continue as a public npm package

Optional Improvements

  1. Consider adding a migration guide in the PR description
  2. Run build tests for both apps/web and apps/memory-graph-playground to ensure everything works
  3. Verify the bun.lock configVersion removal was intentional

📊 Summary

Category Status Notes
Code Quality ✅ Good Clean, consistent changes
Completeness ⚠️ Incomplete 1 app not updated
Security ✅ No Issues No vulnerabilities introduced
Performance ✅ No Impact Import-only changes
Documentation ⚠️ Needs Update Docs still reference old package
Tests ⚠️ None Added Consider adding import tests

Overall Assessment: The changes are well-executed for the web app but incomplete. Please update the memory-graph-playground app and consider updating the documentation before merging.


Let me know if you'd like help with the required changes! 🚀

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