Skip to content

Conversation

@MaheshtheDev
Copy link
Member

@MaheshtheDev MaheshtheDev commented Jan 15, 2026

deep-research on user profile

add novel integration

tiptap 3.x integration

@MaheshtheDev MaheshtheDev marked this pull request as ready for review January 15, 2026 20:51
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.

@cloudflare-workers-and-pages
Copy link

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)
🔵 In progress
View logs
supermemory-app 9aa85c5 Jan 15 2026, 08:51 PM

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 15, 2026

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 59c294b Jan 16 2026, 04:36 AM

@claude
Copy link

claude bot commented Jan 15, 2026

Pull Request Review: Deep Research & TipTap 3.x Integration

Overview

This PR adds significant new functionality including deep research capabilities for user onboarding and TipTap 3.x text editor integration for notes. The implementation is generally solid, but there are several areas that need attention.


🔴 Critical Issues

1. Missing API Key Validation (apps/web/app/api/onboarding/research/route.ts)

The research endpoint uses XAI_API_KEY but there's no validation that the key exists before making the API call.

Issue: If the key is missing, the error will only surface at runtime with a generic 500 error.

Recommendation:

export async function POST(req: Request) {
	try {
		if (!process.env.XAI_API_KEY) {
			return Response.json(
				{ error: "AI service is not configured" },
				{ status: 503 }
			)
		}
		// ... rest of the code

2. URL Validation Vulnerability (apps/web/app/api/onboarding/research/route.ts:38-43)

The URL validation is too permissive. It uses includes() checks which can be bypassed.

Issue:

if (!lowerUrl.includes("x.com") && !lowerUrl.includes("twitter.com"))

This allows URLs like: https://malicious.com/x.com/fake or https://evil.com?redirect=twitter.com

Recommendation:

const urlPattern = /^https?:\/\/(www\.)?(x\.com|twitter\.com)\//i
if (!urlPattern.test(lowerUrl)) {
	return Response.json(
		{ error: "URL must be an X/Twitter profile link" },
		{ status: 400 }
	)
}

3. Handle Extraction Logic Issue (apps/web/app/api/onboarding/research/route.ts:67)

The handle extraction uses simple string replacement which can fail with complex URLs.

Issue:

includedXHandles: [lowerUrl.replace("https://x.com/", "").replace("https://twitter.com/", "")]

This doesn't handle URLs like https://x.com/username/status/123 properly.

Recommendation: Use the existing parseXHandle function from lib/url-helpers.ts instead.


⚠️ Security Concerns

4. No Rate Limiting on Research Endpoint

The research endpoint calls an external AI service without rate limiting, which could be expensive if abused.

Recommendation: Add rate limiting or authentication checks. According to CLAUDE.md, the app supports "API key authentication for external access" - consider requiring authentication.

5. PII Exposure (apps/web/app/api/onboarding/research/route.ts:46-47)

User email and name are included in the AI prompt without sanitization.

Impact: Low risk, but email addresses could leak into AI provider logs.

Recommendation: Add a note about data privacy or sanitize PII before sending.


🟡 Code Quality & Best Practices

6. Missing Error Messages (apps/web/app/api/onboarding/research/route.ts:78)

The catch block only logs errors and returns a generic message.

Issue:

catch (error) {
	console.error("Research API error:", error)
	return Response.json({ error: "Internal server error" }, { status: 500 })
}

Recommendation: Follow the pattern from CLAUDE.md which mentions "HTTPException for consistent API error responses". Consider using Sentry integration more effectively here.

7. Content Change Detection Logic (apps/web/components/new/text-editor/index.tsx:34-39)

The debounced updates check hasUserEditedRef to prevent initial updates, but this could miss legitimate programmatic changes.

Potential Issue: If content is programmatically updated after user edits, it won't trigger onContentChange.

Consideration: Document this behavior or reconsider the logic.

8. TipTap Content Parsing (apps/web/components/new/document-cards/note-preview.tsx:23-49)

The extractTextFromTipTapContent function assumes markdown format but TipTap can store JSON.

Issue: The content from the editor might be markdown (based on line 37: contentType: "markdown"), but the function tries to parse it as JSON first, which will fail for markdown strings.

Recommendation:

  • Either store content as JSON consistently, or
  • Check content format before parsing, or
  • Store both markdown and JSON versions

9. Font Import Path Changes

The PR changes import paths from @/utils/fonts to @/lib/fonts across 20+ files but doesn't show the old file being deleted.

Recommendation: Ensure apps/web/utils/fonts.ts is properly removed to avoid confusion.


🔵 Performance Considerations

10. Debounce Delay (apps/web/components/new/text-editor/index.tsx:34)

500ms debounce for content updates is reasonable, but might feel sluggish for users expecting instant saves.

Suggestion: Consider making this configurable or reducing to 300ms.

11. Editor Destruction (apps/web/components/new/text-editor/index.tsx:109-112)

The editor is properly destroyed on unmount, but the debounced callback isn't cancelled.

Issue: If the component unmounts during the debounce delay, the callback could fire after cleanup.

Recommendation:

useEffect(() => {
	return () => {
		debouncedUpdates.cancel()
		editor?.destroy()
	}
}, [editor, debouncedUpdates])

12. Slash Command Suggestions (apps/web/components/new/text-editor/slash-command.tsx)

The file wasn't included in the diff but is referenced. This is a new 308-line file that should be reviewed for:

  • Command injection vulnerabilities
  • Performance of the suggestion filtering
  • Memory leaks in the React portal usage

🟢 Positive Observations

✅ Well-structured URL helper functions

The new lib/url-helpers.ts file is well-organized with clear JSDoc comments and comprehensive input handling.

✅ Proper TypeScript usage

The TipTap integration uses proper TypeScript types from @tiptap/core.

✅ Accessibility improvements

The document modal close button includes proper aria attributes and the cursor-pointer class.

✅ Good UX patterns

  • The unsaved changes indicator with floating save button is a nice touch
  • BubbleMenu for formatting is intuitive
  • Proper focus management in the editor

📋 Missing Test Coverage

Based on CLAUDE.md's emphasis on quality, these critical paths need tests:

  1. Research API endpoint (apps/web/app/api/onboarding/research/route.ts)

    • URL validation
    • Handle extraction
    • Error handling
    • API key validation
  2. URL helper functions (apps/web/lib/url-helpers.ts)

    • parseXHandle() with various input formats
    • parseLinkedInHandle() edge cases
    • collectValidUrls() filtering logic
  3. TipTap editor integration

    • Content change detection
    • Markdown/JSON parsing
    • Save functionality
    • Keyboard shortcuts
  4. Document modal

    • Unsaved changes detection
    • Save/cancel flows
    • Content reset on modal close

📝 Minor Issues

  • Line 67 (apps/web/app/api/onboarding/research/route.ts): The includedXHandles could be an array of multiple handles, but only one is provided
  • Unused imports: Check if all imported dependencies are actually used (e.g., dompurify in package.json)
  • Missing .env.example documentation: The new API keys should have comments explaining where to obtain them
  • Console.log left in code (apps/web/components/new/onboarding/setup/chat-sidebar.tsx:46): Should be removed or replaced with proper logging

🎯 Recommendations Summary

Must Fix Before Merge

  1. Fix URL validation vulnerability ([ImgBot] Optimize images #2)
  2. Add API key validation (Gemini api #1)
  3. Fix handle extraction logic ((new ui) window is not defined in useViewport #3)

Should Fix

  1. Add rate limiting to research endpoint
  2. Fix debounce cleanup issue (Claude, Gemini, Openai Support (Switch models option) #11)
  3. Improve error handling with Sentry context

Nice to Have

  1. Add test coverage for critical paths
  2. Remove console.log statements
  3. Document new environment variables
  4. Consider performance optimizations

Conclusion

This PR adds valuable functionality with generally clean code. The TipTap 3.x integration is well-implemented, and the URL helper functions are comprehensive. However, the research API endpoint has security vulnerabilities that must be addressed before merging. Once the critical issues are resolved and test coverage is added, this will be a solid addition to the codebase.

Estimated effort to address critical issues: 2-3 hours
Recommended next steps:

  1. Fix URL validation
  2. Add API key checks
  3. Add unit tests for URL helpers and research endpoint
  4. Manual testing of the onboarding flow with the deep research feature

Review generated with attention to the repository's CLAUDE.md guidelines on security, best practices, and code quality.

@MaheshtheDev MaheshtheDev changed the title feat: deep-research on user profile feat: deep-research on user profile and tiptap integration Jan 15, 2026
@graphite-app
Copy link

graphite-app bot commented Jan 15, 2026

Merge activity

deep-research on user profile

add novel integration

tiptap 3.x integration
@graphite-app graphite-app bot force-pushed the 01-14-feat_deep-research_on_user_profile branch from 6d68754 to 59c294b Compare January 15, 2026 21:53
@graphite-app graphite-app bot merged commit 59c294b into main Jan 15, 2026
3 of 6 checks passed
const debouncedUpdates = useDebouncedCallback((editor: Editor) => {
if (!hasUserEditedRef.current) return
const json = editor.getJSON()
const markdown = editor.storage.markdown?.manager?.serialize(json) ?? ""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The markdown serialization uses an incorrect TipTap API path, editor.storage.markdown?.manager?.serialize(json). This will silently fail and cause data loss when editing documents.
Severity: CRITICAL

Suggested Fix

Replace the incorrect API call editor.storage.markdown?.manager?.serialize(json) with the correct method for the installed TipTap version, which is editor.markdown.serialize(json) or editor.getMarkdown().

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/web/components/new/text-editor/index.tsx#L37

Potential issue: The component uses an incorrect API path to serialize editor content to
Markdown. It calls `editor.storage.markdown?.manager?.serialize(json)`, but the official
TipTap v3 documentation specifies using `editor.markdown.serialize(json)` or
`editor.getMarkdown()`. Due to optional chaining (`?.`) and a nullish coalescing
operator (`?? ""`), this error fails silently. On every editor update, the `markdown`
variable becomes an empty string, causing the `onContentChange` callback to report empty
content. This results in user edits being lost upon saving the document.

Did we get this right? 👍 / 👎 to inform future reviews.

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