chore(sdk): conversation id param for memory generation

[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	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	supermemory-app	1cf60d2	Commit Preview URL	Dec 15 2025, 04:17 AM

[MaheshtheDev]

• chore(sdk): conversation id param for memory generation #617 👈 (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]

Code Review: Conversation ID Parameter Refactoring

Summary

This PR refactors the withSupermemory function to move conversationId from an optional parameter to a required positional parameter. This is a breaking change that improves the API clarity.

---

✅ Positive Changes

1. Improved API Design: Moving conversationId to a required positional parameter makes its importance more explicit and reduces nesting in the options object.

2. Better Type Safety: Using string | null explicitly handles the case where conversation tracking isn't needed, which is clearer than string | undefined.

3. Consistent Documentation: The JSDoc comments have been updated to reflect the new signature, and the example code correctly demonstrates the new usage.

4. Proper Null Checks: The middleware now correctly checks for both conversationId existence and truthiness before attempting to add memories (middleware.ts:118, 163).

---

⚠️ Issues & Concerns

1. Breaking Change Without Version Bump Strategy

• The version was bumped from 1.3.13 to 1.3.50 (a significant jump)
• This is a breaking API change that should ideally be a major version bump (e.g., 2.0.0)
• The large version jump (37 versions) is unusual and may indicate synchronization issues

Recommendation: Consider using semantic versioning properly:

• Major version bump for breaking changes
• Or document the migration path clearly in release notes

2. Missing Test Updates

The test file test/vercel.test.ts has not been updated to reflect the new API signature. Multiple test cases will now fail:

• Line 71: withSupermemory(mockModel, TEST_CONFIG.containerTag) - missing conversationId parameter
• Line 79: Same issue
• Line 99-102: createSupermemoryMiddleware calls missing parameters
• Lines 124, 151, 189, 220, 249, 284, 310, 354, 390, 444, 464, 480, 506, 530: All middleware creation calls need updating

Critical: Tests must be updated to:

// Old
withSupermemory(mockModel, TEST_CONFIG.containerTag, { conversationId: "test-123" })

// New
withSupermemory(mockModel, TEST_CONFIG.containerTag, "test-123", { })

3. Type Definition in addMemoryTool Inconsistency

In middleware.ts:37, the parameter type is:

conversationId: string | undefined

But the middleware function signature uses string | null. While these work similarly, using null consistently would be better for clarity.

Recommendation: Change line 37 to:

conversationId: string | null

4. Incomplete Migration Guide

Users upgrading from 1.3.13 to 1.3.50 will experience immediate breakage. Consider:

• Adding a migration guide in the commit message or PR description
• Documenting the breaking change clearly
• Providing a codemod or clear before/after examples

---

🔒 Security Considerations

✅ No security concerns identified. The change doesn't introduce new attack vectors.

---

⚡ Performance Considerations

✅ No performance impact. The refactoring is purely structural.

---

📋 Test Coverage

❌ Critical Issue: Existing tests are not updated and will fail with the new API signature.

Action Required:

1. Update all test files to use the new API signature
2. Run bun run test to verify all tests pass
3. Consider adding tests for the null conversation ID case

---

🎯 Recommendations

Before merging:

1. Update all tests in test/vercel.test.ts to use the new signature
2. Run the test suite and ensure all tests pass
3. Document the breaking change clearly in the PR description
4. Consider the version number - should this be 2.0.0 instead of 1.3.50?
5. Fix type consistency - use string | null throughout

Optional improvements:

• Add a test case specifically for conversationId: null to verify memory isn't added
• Add a test case to verify the customId is correctly set when conversationId is provided
• Update any related documentation or README files

---

📊 Overall Assessment

Code Quality: Good refactoring that improves API clarity
Breaking Change: Yes - requires major version or clear migration path
Test Coverage: ❌ Failing - tests must be updated
Ready to Merge: ❌ Not yet - tests need updates

The core refactoring is solid and improves the API design, but the PR needs test updates before it can be safely merged.

[Dhravya]

@MaheshtheDev why draft? Also this would be a version bump since it changes the schema

[claude]

Pull Request Review: Conversation ID Parameter Refactor

Summary

This PR refactors the conversationId parameter from being an optional field in the options object to a required positional parameter in the withSupermemory function. This is a breaking API change that improves clarity and consistency.

---

✅ Code Quality & Best Practices

Strengths:

1. Improved API Design: Making conversationId a positional parameter is clearer and more intuitive. The new signature withSupermemory(model, containerTag, conversationId, options) makes it obvious that conversation ID is a first-class concern alongside container tag.

2. Consistent Documentation: The JSDoc comments are properly updated to reflect the new parameter position, and the example code is corrected.

3. Type Safety: Changed from string | undefined to string | null, which is more explicit about the intentional absence of a value vs. unintentional undefined.

4. Proper Null Checking: The middleware now correctly checks for both existence and non-null before using the conversation ID:

   if (addMemory === "always" && userMessage && userMessage.trim() && conversationId)

---

⚠️ Issues & Concerns

1. Breaking Change Without Version Bump Strategy

• Issue: The version bump from 1.3.13 to 1.3.50 is unusual and doesn't follow semantic versioning.
• Impact: This is a breaking change that should ideally be a major version bump (2.0.0) or at minimum a minor version bump (1.4.0).
• Current users will experience breaking changes when they update, as the API signature has changed:

  // Old (no longer works)
  withSupermemory(model, "user-123", { conversationId: "conv-456" })
  
  // New (required)
  withSupermemory(model, "user-123", "conv-456", {})

2. Missing Test Updates

• Issue: The existing test suite at /packages/tools/test/vercel.test.ts does not appear to have been updated to reflect the new API signature.
• Evidence: The test file still references the old signature pattern where conversationId was in options.
• Risk: Tests may fail or not properly validate the new behavior.
• Recommendation: Update all tests to use the new signature and add specific tests for:
  • conversationId as null
  • conversationId as a valid string
  • Memory generation with and without conversationId

3. README Documentation Inconsistency

• Issue: The README.md shows the old API in line 86-88:

  const modelWithMemory = withSupermemory(openai("gpt-5"), "user_id_life", {
    conversationId: "conversation-456"
  })

  But the PR changes it to:

  const modelWithMemory = withSupermemory(openai("gpt-4"), "user-123", "conversation-456", {
    mode: "full",
    addMemory: "always"
  })

• Impact: Users following the README examples will encounter errors.
• Recommendation: Update all README examples to use the new signature.

4. Type Definition Missing

• Issue: The WrapVercelLanguageModelOptions interface in the diff still shows conversationId?: string (line 6 in index.ts), but it was removed from the interface.
• Concern: Need to verify that the TypeScript types are properly exported and match the new implementation.

---

🔒 Security Considerations

No security concerns identified. The change doesn't introduce any new attack vectors:

• Conversation IDs are still properly validated before use
• No sensitive data exposure
• Proper null checking prevents potential undefined behavior

---

⚡ Performance Considerations

Neutral impact. The refactor:

• Doesn't change the underlying logic flow
• Still performs the same null/existence checks
• No new async operations or data processing

---

🧪 Test Coverage

Needs improvement:

1. Current test coverage is outdated - Tests need to be updated to match new signature
2. Missing test cases for the new API:
   • Passing null explicitly for conversationId
   • Passing valid conversationId string
   • Verifying memory generation is skipped when conversationId is null
   • Verifying memory generation succeeds when conversationId is provided

Suggested test additions:

it("should skip memory generation when conversationId is null", async () => {
  const middleware = createSupermemoryMiddleware(
    TEST_CONFIG.containerTag,
    TEST_CONFIG.apiKey,
    null, // explicitly null
    false,
    "profile",
    "always"
  )
  // Assert memory is not saved
})

it("should generate memory when conversationId is provided", async () => {
  const middleware = createSupermemoryMiddleware(
    TEST_CONFIG.containerTag,
    TEST_CONFIG.apiKey,
    "conv-123", // valid conversation ID
    false,
    "profile",
    "always"
  )
  // Assert memory is saved with conversation context
})

---

📋 Recommendations

Before merging:

1. ✅ Update README.md - Change all examples to use the new API signature
2. ✅ Update test suite - Modify test/vercel.test.ts to use new signature and add missing test cases
3. ⚠️ Reconsider version bump - Consider using semantic versioning (2.0.0 for breaking change)
4. ✅ Add migration guide - Document the breaking change in release notes or CHANGELOG
5. ⚠️ Verify middleware.ts signature - Ensure the exported function signature is correctly typed

Migration guide for users:

// Before (v1.3.x)
const model = withSupermemory(openai("gpt-4"), "user-123", {
  conversationId: "conv-456",
  mode: "full"
})

// After (v1.3.50+)
const model = withSupermemory(openai("gpt-4"), "user-123", "conv-456", {
  mode: "full"
})

// If no conversation ID needed:
const model = withSupermemory(openai("gpt-4"), "user-123", null, {
  mode: "full"
})

---

📊 Overall Assessment

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

• Well-structured refactor with improved API ergonomics
• Proper null handling and type safety

Completeness: ⭐⭐⭐ (3/5)

• Missing test updates
• Documentation needs updates
• Breaking change not properly versioned

Recommendation: ✅ Approve with changes required

This is a good improvement to the API, but needs test updates and documentation fixes before merging. The breaking change should also be clearly communicated to users.