Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-mcp | 579ef03 | Apr 14 2026, 03:21 PM |
MaheshtheDev
left a comment
There was a problem hiding this comment.
@sreedharsreeram please update the package version and update the documentation for this packages with these new tools
5eb9bde to
6bb8c68
Compare
How to use the Graphite Merge QueueAdd 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. |
Done! Please review |
|
@sreedharsreeram please fix merge conflicts |
6bb8c68 to
3bdf7d3
Compare
Confidence Score: 2/5 - Changes NeededNot safe to merge — the new Key Findings:
Files requiring special attention
|
|
Claude finished @sreedharsreeram's task in 1m 36s —— View job PR Review: Vercel AI SDK backfilling
Summary: This PR adds hybrid/documents search modes for RAG via a new Verdict: Ready to ship (9/10) All previously flagged issues have been addressed:
See my full review for details. |
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG and makes conversationId a required parameter.
Issues found:
-
Critical: ReferenceError in
memory-client.ts:329—memoriesResponseis referenced outside its scope. WhenuseHybridSearchis true (hybrid/documents mode), this will throw a ReferenceError at runtime sincememoriesResponseis only defined in theelsebranch. -
Documentation mismatch — All README examples use the old 3-argument signature
withSupermemory(model, containerTag, options)but the API now requires 4 arguments withconversationIdas required positional parameter. -
Breaking change flagged by other reviewers — The
withSupermemorysignature change from(model, containerTag, options?)to(model, containerTag, conversationId, options?)is a breaking change that should warrant a major version bump (2.0.0), not minor (1.5.0).
Score: 5/10
The hybrid search feature looks well-designed, but the ReferenceError will crash any usage of hybrid/documents mode. This needs to be fixed before merge.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — the Key Findings:
Files requiring special attention
|
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces two critical runtime-breaking bugs that will cause immediate failures in production. In Key Findings:
Files requiring special attention
|
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG and makes conversationId a required parameter.
Issues found:
-
Critical: ReferenceError in
memory-client.ts:335—memoriesResponseis referenced outside its scope. WhenuseHybridSearchis true (hybrid/documents mode), this will throw a ReferenceError at runtime sincememoriesResponseis only defined in theelseblock at line 274. -
Documentation mismatch — All README examples (lines 193, 220) use the signature without the now-required
conversationIdfield. Sinceoptionsis now required withconversationId: string, examples likewithSupermemory(openai("gpt-4"), "user-123", { mode: "full" })will fail TypeScript compilation. -
Breaking change — Making
optionsrequired andconversationIdrequired within it is a breaking change. TypeScript will catch this at compile time (not a silent failure), but it warrants a major version bump (2.0.0) rather than minor (1.5.0).
Score: 4/10
The hybrid search feature is well-designed, but the ReferenceError will crash any usage of hybrid/documents mode. This must be fixed before merge.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR contains a confirmed runtime Key Findings:
Files requiring special attention
|
Confidence Score: 4/5 - Mostly SafeLikely safe but review recommended — this PR introduces Key Findings:
Files requiring special attention
|
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG and makes conversationId required.
Good news: The ReferenceError bug flagged by earlier reviewers has been fixed in this version. The code now properly declares rawSearchResults at function scope (line 216), avoiding the scoping issue where memoriesResponse was referenced outside its else block.
Issues found:
-
Documentation examples missing required field — All new examples in README.md and user-profiles.mdx are missing the now-required
conversationIdfield. Users following the docs will get TypeScript errors. See inline comments for suggested fixes. -
Breaking API change — Making
optionsrequired andconversationIdrequired within it changes the public API signature fromwithSupermemory(model, tag, options?)towithSupermemory(model, tag, options)where options must includeconversationId. This is technically a breaking change that would warrant a major version bump (2.0.0) under strict semver. However, TypeScript will catch this at compile time (not a silent failure), so the impact is manageable.
Score: 8/10
The hybrid search feature is well-implemented and the previous scoping bug has been fixed. The only blocking issues are the documentation examples that won't compile. Once those are updated, this is good to ship.
Confidence Score: 4/5 - Mostly SafeSafe to merge — this PR cleanly introduces hybrid search support via a new Key Findings:
Files requiring special attention
|
Confidence Score: 4/5 - Mostly SafeSafe to merge — this PR cleanly introduces hybrid search (RAG) capability and conversation-scoped memory enforcement to Key Findings:
Files requiring special attention
|
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG via a new /v4/search endpoint, enabling search across both memories AND document chunks.
Issues found: None — this looks good to ship.
Clarifications on previous review concerns:
-
ReferenceError (FIXED): The scoping bug where
memoriesResponsewas referenced outside itselseblock has been properly fixed. The code now declaresrawSearchResultsat function scope (memory-client.ts:216-219), so it's always accessible regardless of which branch executes. -
Breaking API change (NOT an issue): The
withSupermemorysignature remains(model, containerTag, options?)withconversationIdstaying optional insideWrapVercelLanguageModelOptions(vercel/index.ts:17). The only change is a new runtime validation: ifaddMemory: "always"is set, thenconversationIdis required (vercel/index.ts:131-138). This is defensive programming — TypeScript will catch it at compile time if you have proper types, and runtime throws a clear error message. This is not a breaking change for existing callers. -
Documentation (CORRECT): The hybrid search examples correctly omit
conversationIdwhen not usingaddMemory: "always". Examples that do useaddMemory: "always"now includeconversationId.
What's well done:
- Clean separation between hybrid search path and legacy profile search path in
buildMemoriesText - Proper error handling in both
supermemoryHybridSearchandsupermemoryProfileSearch - Good TypeScript types for
SearchModeandSearchResultItem - Clear JSDoc documentation throughout
- Appropriate minor version bump (1.5.0) for additive, non-breaking features
Score: 9/10
Confidence Score: 4/5 - Mostly SafeLikely safe but review recommended — this PR introduces Key Findings:
Files requiring special attention
|
Prefix unused variables with underscore and fix formatting in memory-client. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Break long conditionals across multiple lines and use single quotes for string containing escaped double quotes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
730a790 to
a0d469a
Compare
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG via a new /v4/search endpoint, enabling search across both memories AND document chunks.
Issues found: None — this looks good to ship.
Clarifications on previous review concerns:
-
ReferenceError (FIXED): The scoping bug where
memoriesResponsewas referenced outside itselseblock has been properly fixed. The code now declaresrawSearchResultsat function scope (memory-client.ts:216-219), and it's correctly assigned in both the hybrid search branch (line 255) and the profile search branch (line 339). -
Breaking API change (NOT an issue): The
withSupermemorysignature remains(model, containerTag, options?)withconversationIdstaying optional insideWrapVercelLanguageModelOptions(vercel/index.ts:17). The new runtime validation at lines 131-138 only throws ifaddMemory: "always"is set withoutconversationId— this is correct defensive programming, not a breaking change. Existing callers are unaffected. -
Documentation: Updated correctly with new
searchModeandsearchLimitoptions. Examples that useaddMemory: "always"now includeconversationId.
What's well done:
- Clean separation between hybrid search path and legacy profile search path in
buildMemoriesText - Proper error handling in both
supermemoryHybridSearchandsupermemoryProfileSearch - Good TypeScript types for
SearchModeandSearchResultItem - Clear JSDoc documentation throughout
- Appropriate minor version bump (1.5.0) for additive, non-breaking features
Score: 9/10
324179c to
147a549
Compare
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG via a new /v4/search endpoint and refactors the withSupermemory API to use an options object pattern.
Issues found:
-
Breaking API change without README update — The
withSupermemorysignature changed from(model, containerTag, options?)to({ model, containerTag, customId, ...options }). The README still shows the old positional argument API, which will cause runtime errors for anyone following the docs. -
addMemorydefault changed from "never" to "always" — This is a significant behavior change. Existing users who upgrade will suddenly have all conversations saved to memory without opting in. This should either be reverted to maintain backwards compatibility or explicitly documented as a breaking change. -
customIdis now required — PreviouslyconversationIdwas optional. NowcustomIdis required in the options object. This needs to be reflected in all documentation.
What's working well:
- The ReferenceError bug flagged by earlier reviewers has been properly fixed (
rawSearchResultsis now declared at function scope) - Clean implementation of hybrid search with proper TypeScript types
- Good test coverage for the new caching behavior
Score: 7/10
The hybrid search feature is well-implemented, but the undocumented breaking changes and the addMemory default flip need to be addressed before merge. Either update all README examples to match the new API, or consider a major version bump to signal the breaking changes.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG via a new /v4/search endpoint and refactors withSupermemory to use an options object pattern.
Issues found: None — this looks good to ship.
Clarifications on previous review concerns:
-
ReferenceError (FIXED): The scoping bug where
memoriesResponsewas referenced outside itselseblock has been properly fixed. The code now declaresrawSearchResultsat function scope (memory-client.ts:216-219) and correctly assigns it in both the hybrid search branch (line 255) and the profile search branch (line 339). -
Breaking API change (Documented): The
withSupermemorysignature changed from positional args(model, containerTag, options?)to an options object({ model, containerTag, customId, ...options }). This is a breaking change, but:- All tests have been updated to use the new pattern
- README documentation reflects the new API signature
- The options object pattern is cleaner and more extensible
-
addMemorydefault: The default is"always"as documented in the README under "Automatic Memory Capture". This is set explicitly invercel/index.ts:155.
What's well done:
- Clean separation between hybrid search path and legacy profile search in
buildMemoriesText - Proper error handling with typed responses from
supermemoryHybridSearch - Good TypeScript types for
SearchMode,SearchResultItem, andSearchResponse - Per-turn memory caching to avoid redundant API calls during tool-call continuations
- Comprehensive test coverage for the new options pattern
Score: 9/10
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG via a new /v4/search endpoint and refactors withSupermemory to use an options object pattern with customId as a required field.
Issues found: None — this looks good to ship.
Clarifications on previous review concerns:
-
ReferenceError (FIXED): The scoping bug where
memoriesResponsewas referenced outside itselseblock has been properly fixed. The code now declaresrawSearchResultsat function scope (memory-client.ts:216-219) and correctly assigns it in both the hybrid search branch (line 255) and the profile search branch (line 339). -
Breaking API change (Documented): The
withSupermemorysignature changed from positional args to an options object{ model, containerTag, customId, ...options }. This is a breaking change, but:- README documentation has been fully updated to show the new pattern
- All tests have been updated to use the new API
- Proper validation with clear error messages for missing
customId
-
Documentation: README correctly shows
customIdas required and documents all new options includingsearchModeandsearchLimit.
What's well done:
- Clean separation between hybrid search path and legacy profile search in
buildMemoriesText - Proper error handling with typed
SearchResponseandSearchResultIteminterfaces - Per-turn memory caching via
MemoryCacheto avoid redundant API calls during tool-call continuations - Comprehensive test coverage for the new options pattern
Score: 9/10
There was a problem hiding this comment.
Overview: Adds hybrid/documents search modes for RAG via a new /v4/search endpoint and refactors withSupermemory to use an options object pattern with customId as a required field.
Issues found: None — this looks good to ship.
Verification of previously flagged concerns:
-
ReferenceError (FIXED): The scoping bug where
memoriesResponsewas referenced outside itselseblock has been properly fixed. The code now declaresrawSearchResultsat function scope (memory-client.ts:216-219) and correctly assigns it in both the hybrid search branch (line 255) and the profile search branch (line 339). ThepromptDataconstruction at line 345 uses this function-scoped variable. -
Breaking API change (Documented): The
withSupermemorysignature changed from positional args(model, containerTag, options?)to an options object{ model, containerTag, customId, ...options }. This is properly reflected in:- README.md with all examples updated
- All integration tests using the new pattern
- docs-test files updated
- Proper validation with clear error messages for missing
customId
-
Hybrid search implementation: Clean separation between hybrid search path (uses
/v4/search) and legacy profile search path (uses/v4/profile) inbuildMemoriesText. Error handling is consistent across both paths.
What's well done:
- Per-turn memory caching via
MemoryCacheto avoid redundant API calls during tool-call continuations - Proper TypeScript types for
SearchMode,SearchResultItem, andSearchResponse - Comprehensive test coverage for the new options pattern
- Clear JSDoc documentation throughout
Score: 9/10

No description provided.