fix(docs): delete connections sdk snippet

[MaheshtheDev]

No description provided.

[cloudflare-workers-and-pages]

Deploying with    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	ea9fa65	Dec 24 2025, 01:46 AM

[MaheshtheDev]

• fix(docs): delete connections sdk snippet #629 👈 (View in Graphite)
• main

---

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]

Pull Request Review

Thank you for working on fixing the SDK documentation! This PR correctly updates the method names from delete() to deleteByID() and adds the deleteByProvider() examples. Here's my detailed review:

✅ What's Good

1. Correct Method Names: The changes properly update to deleteByID() (TypeScript) and delete_by_id() (Python), which appear to be the correct method signatures
2. Added Missing Functionality: Great addition of the deleteByProvider() examples with container tags - this was missing before
3. Consistent Formatting: Code examples are well-formatted and clear
4. Both Languages Updated: Both TypeScript and Python examples are updated consistently

⚠️ Issues Found

1. Incomplete Fix - Other Connector Docs Still Have Old Method Names

This PR only fixes the GitHub connector docs, but I found the same issue in other connector documentation files:

Still using old delete() method (needs fixing):

• apps/docs/connectors/notion.mdx:190 and :203
• apps/docs/connectors/onedrive.mdx:209 and :221
• apps/docs/connectors/web-crawler.mdx:259 and :272

These files already have the deleteByProvider() method documented correctly, but still use the old delete() instead of deleteByID()/delete_by_id().

Already correct (using deleteByID()):

• apps/docs/connectors/google-drive.mdx ✅
• apps/docs/connectors/overview.mdx ✅
• apps/docs/connectors/troubleshooting.mdx ✅

2. Minor: Removed Example Output Comments

The PR removes these helpful comments:

// Output: Deleted: conn_abc123 notion

While this is a minor point, these example outputs can be helpful for developers to understand what to expect. Consider keeping them if they're accurate.

📋 Recommendations

1. Extend this fix to all connector docs - Apply the same delete() → deleteByID()/delete_by_id() changes to:

   • apps/docs/connectors/notion.mdx
   • apps/docs/connectors/onedrive.mdx
   • apps/docs/connectors/web-crawler.mdx

2. Consider adding a test - If the SDK has tests, ensure there are tests verifying both deleteByID() and deleteByProvider() work correctly

3. Optional: Add a note about container tags - The deleteByProvider() method requires container tags. Consider adding a brief explanation of why they're required (for scoping to specific users/contexts)

🎯 Action Items

• Apply the same fix to Notion, OneDrive, and Web Crawler connector docs
• Verify the actual SDK implementation has these methods with these exact names
• Consider keeping or removing example output comments consistently across all docs

Code Quality: ⭐⭐⭐⭐ (4/5)

The changes themselves are correct and well-formatted. Deducting one star only because the fix is incomplete (other docs need the same update).

Security: ✅ No concerns

Documentation changes only, no security implications.

Performance: ✅ No concerns

Documentation changes only, no performance implications.

Test Coverage: ⚠️ Cannot verify

Unable to locate SDK test files to verify if these methods are tested. The SDK source files don't appear to be in this monorepo (they may be in a separate repository).

Great work on catching this documentation bug! 🎉

[graphite-app]

Merge activity

• Dec 24, 1:44 AM UTC: Dhravya added this pull request to the Graphite merge queue.
• Dec 24, 1:46 AM UTC: Merged by the Graphite merge queue.

[claude]

Pull Request Review

Summary

This PR updates the SDK documentation for deleting connections, changing from the generic delete() method to more specific deleteByID() and deleteByProvider() methods. This improves API clarity and provides users with more explicit options.

✅ Positive Aspects

1. Improved API Clarity: The change from delete() to deleteByID() makes the method's purpose explicit, following better naming conventions
2. Added Flexibility: Introducing deleteByProvider() gives users an alternative way to delete connections using provider names and container tags
3. Consistent Documentation: Both TypeScript and Python examples are updated consistently
4. Better Examples: The examples now show both deletion methods, helping users choose the right approach

⚠️ Issues and Concerns

1. Documentation Inconsistency Across Files

The most critical issue is that this PR only updates 2 files (github.mdx and overview.mdx), but the following files still use the old delete() method:

• apps/docs/connectors/notion.mdx (line 190, 203) - Uses delete(connection_id) instead of deleteByID()
• Other connector docs may have similar issues

Impact: Users will see inconsistent examples across different connector documentation pages, leading to confusion about the correct API usage.

Recommendation: Update ALL connector documentation files to use the new method names consistently.

2. Missing Migration/Deprecation Information

The documentation doesn't mention:

• Is the old delete() method deprecated?
• Will it continue to work?
• Is this a breaking change for existing SDK users?

Recommendation: Add a note in the documentation or changelog about this API change.

3. Removed Example Output Comments

The PR removes these helpful lines:

// Output: Deleted: conn_abc123 notion

Impact: Minor - these comments helped users understand what to expect, but aren't critical.

4. SDK Implementation Verification Missing

The review cannot verify if the actual SDK (npm package supermemory) has been updated with these method names, since the SDK code isn't in this repository.

Critical Questions:

• Does the supermemory npm package already have deleteByID() and deleteByProvider() methods?
• Has the SDK been published with these changes?
• What version of the SDK includes these methods?

Recommendation: Verify the SDK implementation before merging docs, or add version information to the docs (e.g., "Available in SDK v3.1.0+").

🔍 Code Quality

• ✅ Syntax is correct for both TypeScript and Python examples
• ✅ Indentation and formatting are consistent
• ✅ The API signatures make sense and follow common patterns

🧪 Test Coverage

N/A - This is documentation only, but consider:

• Adding integration tests that verify the SDK examples actually work
• CI checks to ensure all docs stay in sync

🎯 Recommendations

Before merging:

1. Critical: Update notion.mdx and all other connector docs to use the new method names
2. Important: Verify the SDK package has these methods implemented and published
3. Nice-to-have: Add a note about SDK version requirements or migration path

After merging:

1. Consider adding automated checks to prevent documentation drift across connector files
2. Add a shared documentation snippet for delete operations to maintain consistency

Overall Assessment

The changes improve the API design, but the incomplete update across all documentation files is a blocking issue. This PR should either:

• Update ALL connector documentation files (recommended), or
• Explain why only some files were updated

Status: Request changes - needs comprehensive documentation update across all connector files.

---

Review generated via Claude Code