Skip to content

fix: file upload source#579

Merged
graphite-app[bot] merged 1 commit into
mainfrom
11-12-fix_file_upload_source
Nov 13, 2025
Merged

fix: file upload source#579
graphite-app[bot] merged 1 commit into
mainfrom
11-12-fix_file_upload_source

Conversation

@MaheshtheDev

Copy link
Copy Markdown
Member

No description provided.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 13, 2025

Copy link
Copy Markdown

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 bf225f2 Nov 13 2025, 06:46 AM

@MaheshtheDev MaheshtheDev marked this pull request as ready for review November 13, 2025 06:31
@graphite-app graphite-app Bot requested a review from Dhravya November 13, 2025 06:31

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.

@claude

claude Bot commented Nov 13, 2025

Copy link
Copy Markdown

Pull Request Review: Fix File Upload Source

Summary

This PR adds the sm_source: "consumer" metadata field to file uploads in the Add Memory view. This aligns the file upload flow with other memory creation methods in the codebase.


✅ Positive Aspects

  1. Consistency Fix: This change brings file uploads in line with the existing pattern used throughout the codebase:

    • Text/note creation (line 228) already includes sm_source: "consumer"
    • The subsequent PATCH update (line 450) also includes this metadata
    • Browser extension and other entry points use the same pattern
  2. Proper Placement: The metadata is correctly added to the initial FormData upload request, which is the right approach rather than only adding it in the follow-up PATCH.

  3. Minimal Change: The fix is surgical and focused - only 3 lines added with no modifications to existing logic.


🔍 Code Quality Observations

Good:

  • The code follows the existing pattern exactly
  • Proper JSON serialization of the metadata object
  • Consistent with documentation expectations

Minor Observations:

  • The code at line 450 now has some duplication - both the initial upload (line 423-425) and the follow-up PATCH (line 450) set sm_source: "consumer". This is actually correct for defensive coding, but worth noting.

🐛 Potential Issues

None identified - This is a straightforward bug fix with no apparent issues.


🚀 Performance Considerations

No performance impact - Adding a small metadata field to the FormData has negligible overhead.


🔒 Security Concerns

Low Risk - The value is hardcoded to "consumer", so there's no injection risk. However, some observations:

  1. The comment on line 450 states: "Use 'consumer' source to bypass limits" - this suggests the backend uses sm_source for rate limiting or quota enforcement.
  2. Since the value is hardcoded client-side, a malicious user could theoretically omit or modify this field in direct API calls.
  3. Recommendation: If sm_source is used for critical business logic (limits/quotas), the backend should validate and enforce this field server-side rather than trusting client input.

🧪 Test Coverage

Missing Tests: There are no test files for the add-memory component:

  • No add-memory.test.tsx or add-memory.spec.tsx found
  • This component handles critical functionality (file uploads, memory creation)

Recommendation: Consider adding tests for:

- File upload mutation includes correct metadata
- FormData structure is properly formatted
- Error handling for failed uploads
- Metadata is preserved through the upload flow

📋 Additional Recommendations

  1. Documentation Update: The API documentation (apps/docs/add-memories/examples/file-upload.mdx) doesn't mention the metadata parameter for file uploads. Consider updating the docs to show the complete expected format:
curl -X POST "https://api.supermemory.ai/v3/documents/file" \
  -H "Authorization: Bearer $SUPERMEMORY_API_KEY" \
  -F "file=@document.pdf" \
  -F "containerTags=documents" \
  -F "metadata={\"sm_source\":\"consumer\"}"
  1. Type Safety: Consider creating a type or constant for valid sm_source values:
// packages/validation/schemas.ts or similar
export const SM_SOURCE = {
  CONSUMER: 'consumer',
  TWITTER_BOOKMARKS: 'twitter_bookmarks',
  // ... other sources
} as const;
  1. Consistency Check: Verify that other upload entry points also include this metadata:
    • Raycast extension (apps/raycast-extension/src/api.ts)
    • Any MCP server upload handlers

✅ Verdict

APPROVED - This is a good bug fix that improves consistency across the codebase. The change is minimal, correct, and follows existing patterns.

Merge Recommendation: ✅ Safe to merge

Priority: Medium - This ensures proper source tracking for analytics and potentially for rate limiting logic.


📝 Summary Checklist

  • ✅ Code quality: Good
  • ✅ Bug fixes: Addresses the issue correctly
  • ✅ Performance: No concerns
  • ⚠️ Security: Minor observation about server-side validation
  • ❌ Test coverage: Missing (but pre-existing issue)
  • ⚠️ Documentation: Could be improved

Great work on maintaining consistency! 🎉

@graphite-app

graphite-app Bot commented Nov 13, 2025

Copy link
Copy Markdown

Merge activity

@graphite-app

graphite-app Bot commented Nov 13, 2025

Copy link
Copy Markdown

TV gif. Timmy from Shaun the Sheep blinks and extends 2 thumbs up as a lopsided grin emerges on the side of his face. (Added via Giphy)

@graphite-app graphite-app Bot force-pushed the 11-12-fix_file_upload_source branch from 60e65e7 to bf225f2 Compare November 13, 2025 06:37
@graphite-app graphite-app Bot merged commit bf225f2 into main Nov 13, 2025
3 of 4 checks passed
@claude

claude Bot commented Nov 13, 2025

Copy link
Copy Markdown

Code Review: Fix file upload source metadata

Summary

This PR adds the sm_source: "consumer" metadata field to file uploads, ensuring consistency with other document creation methods in the codebase.


✅ Code Quality & Best Practices

Positive:

  • Consistency: This change aligns the file upload flow with other document creation methods (addContentMutation at line 228 and updateMemoryMutation at line 450)
  • Clean implementation: The metadata is properly JSON stringified as FormData, matching the expected API format
  • Follows existing patterns: Uses the same metadata structure as other parts of the application

Suggestions:

  1. Consider extracting constant: The "consumer" string is repeated across multiple locations. Consider extracting to a shared constant:
    const DOCUMENT_SOURCE = {
      CONSUMER: "consumer",
      TWITTER_BOOKMARKS: "twitter_bookmarks",
    } as const
    This would improve maintainability and reduce typo risks.

🐛 Potential Issues

None identified. The implementation correctly:

  • Uses JSON.stringify() for the metadata object
  • Follows the validated MetadataSchema (record of string/number/boolean values)
  • Matches the FormData structure expected by the API

⚡ Performance Considerations

No concerns. The change:

  • Adds minimal overhead (small JSON object)
  • Uses native FormData API efficiently
  • Doesn't introduce any blocking operations

🔒 Security Concerns

None identified. The implementation:

  • Uses a hardcoded constant ("consumer"), preventing injection risks
  • Follows the authenticated API pattern with credentials: "include"
  • Doesn't expose sensitive user data

🧪 Test Coverage

Gap identified:

  • ❌ No test file exists for add-memory component
  • The file mutation logic (lines 400-454) lacks automated test coverage

Recommendations:

  1. Consider adding integration tests for file uploads that verify:
    • Metadata is correctly included in FormData
    • The sm_source field is set to "consumer"
    • File upload succeeds with proper metadata
  2. Add unit tests for the mutation functions if possible

📝 Additional Notes

Context from codebase analysis:

  • The sm_source field is used throughout the codebase to identify document sources
  • Comment at line 228 indicates this field helps "bypass limits" - ensure this behavior is documented and intentional
  • Other sources include "twitter_bookmarks" (browser extension) and general "consumer" for web app

Related files using sm_source:

  • apps/web/app/onboarding/bio-form.tsx:27
  • apps/browser-extension/entrypoints/background.ts:134
  • apps/browser-extension/utils/twitter-import.ts:158

✨ Verdict

LGTM with suggestions

This is a solid fix that improves consistency across the codebase. The implementation is correct and follows established patterns. Consider the constant extraction suggestion and adding test coverage for future maintainability.

Risk level: Low
Merge confidence: High

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