Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
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. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-app | a73814b | Commit Preview URL | Apr 06 2026, 05:22 PM |
e02d813 to
c9b5b84
Compare
Confidence Score: 2/5 - Changes NeededNot safe to merge — this PR promotes Key Findings:
Files requiring special attention
|
c9b5b84 to
ce566dd
Compare
packages/openai-sdk-python/src/supermemory_openai/middleware.py
Outdated
Show resolved
Hide resolved
Confidence Score: 3/5 - Review RecommendedNot safe to merge without review — while this PR successfully promotes Key Findings:
Files requiring special attention
|
ce566dd to
317ad61
Compare
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces a breaking API change by promoting Key Findings:
Files requiring special attention
|
|
Claude finished @sreedharsreeram's task in 1m 47s —— View job PR Review Complete
Summary: Reviewed the PR making Score: 10/10 — Good to ship. |
There was a problem hiding this comment.
Overview: Makes conversationId a required positional parameter instead of optional config field in both TypeScript and Python OpenAI SDK wrappers.
Issues found: None — this looks good to ship.
The implementation is correct:
- Both SDKs properly validate that
conversationIdis non-empty (rejects empty strings and whitespace-only) - The parameter is correctly propagated through wrapper classes
- Documentation and examples are updated to reflect the new API signature
- Tests cover the new validation behavior and updated signatures
- Shared test utilities extracted to avoid duplication
The breaking change nature has been flagged by existing review comments. The version bumps (1.4.01→1.5.0 for TS, 1.0.2→1.1.0 for Python) are minor versions rather than major, which technically violates semver for breaking changes. However, the implementation itself has no bugs, security issues, or logic errors.
Score: 9/10
Minor deduction for semver semantics (breaking change should ideally be a major version bump), but the code quality is solid.
317ad61 to
a5bb6b6
Compare
Confidence Score: 3/5 - Review RecommendedNot safe to merge without review — while this PR correctly promotes Key Findings:
Files requiring special attention
|
a5bb6b6 to
af4838b
Compare
dea4790 to
0724c75
Compare
There was a problem hiding this comment.
Overview: Makes conversationId a required parameter in both TypeScript and Python OpenAI SDK wrappers, consolidating the API surface.
Issues found: One minor issue already flagged by Sentry — containerTag validation in TypeScript (packages/tools/src/openai/index.ts:75) is missing a .trim() check, allowing whitespace-only strings to pass validation. This is inconsistent with the Python implementation which correctly validates both fields.
The implementation is otherwise solid:
- Both SDKs properly validate that
conversationIdis non-empty with type and.trim()checks - The parameter is correctly propagated through wrapper classes
- Documentation and examples are updated to reflect the new API signature
- Tests cover the new validation behavior and updated signatures
- Shared test utilities extracted to avoid duplication
The breaking change nature is intentional and well-documented. Version bumps are minor (1.5.0 for TS, 1.1.0 for Python) rather than major, which technically doesn't follow semver for breaking changes — but that's a release process decision.
Score: 9/10
Minor deduction for the containerTag validation inconsistency. Code quality is solid with proper validation, error messages, and test coverage.
Confidence Score: 2/5 - Changes NeededNot safe to merge — the new required Key Findings:
Files requiring special attention
|
0724c75 to
591d21e
Compare
There was a problem hiding this comment.
Overview: Consolidates containerTag and customId (formerly conversationId) into a required options object for both TypeScript and Python OpenAI SDK wrappers.
Issues found: One minor issue already flagged by Sentry — containerTag validation in TypeScript (packages/tools/src/openai/index.ts:75) is missing a .trim() check, allowing whitespace-only strings to pass validation. This is inconsistent with the customId validation which correctly uses .trim().
The implementation is otherwise solid:
- Both SDKs properly validate that
customIdis non-empty with type and.trim()checks - Documentation and examples are updated to reflect the new API signature
- Tests cover the new validation behavior and updated signatures
- Shared test utilities extracted to avoid duplication
Note on the Entelligence review claiming conversationId is treated as optional in createOpenAIMiddleware: This is incorrect. The conditional checks like customId ? ... : undefined in the middleware are for determining behavior (e.g., using conversation endpoint vs regular memory endpoint), not for treating it as optional. The validation in withSupermemory correctly enforces customId as required before calling createOpenAIMiddleware.
Score: 9/10
Minor deduction for the containerTag validation inconsistency with .trim(). Code quality is solid with proper validation, error messages, and test coverage.
Confidence Score: 2/5 - Changes NeededNot safe to merge — this v2.0.0 refactor introduces a breaking validation regression where Key Findings:
Files requiring special attention
|
591d21e to
69a6a67
Compare
There was a problem hiding this comment.
Overview: Consolidates containerTag and customId into a required options object for both TypeScript and Python OpenAI SDK wrappers, making the API more explicit and consistent.
Issues found: None — this looks good to ship.
After reviewing the code and existing review comments:
-
Validation is correct: Both TypeScript (
packages/tools/src/openai/index.ts:74-96) and Python (packages/openai-sdk-python/src/supermemory_openai/middleware.py:690-702) properly validate required fields with type checks and.trim()/.strip()to reject whitespace-only strings. -
Sentry's containerTag comment was incorrect: The code at line 77 does include
!options.containerTag.trim()— the validation is consistent withcustomId. -
Responses API behavior is intentional: The
addMemoryToolcall passingundefinedfor messages (line 589) is documented and correct — Responses API doesn't have a messages array, so it falls back to regular memory storage rather than the conversation endpoint. -
The "optional customId" concern is a misread: The conditional checks like
customId ? ... : undefinedin the middleware determine which endpoint/behavior to use, not whethercustomIdis optional. The validation inwithSupermemorycorrectly enforces it as required before the middleware is ever called.
The breaking API change from positional parameters to an options object is intentional and improves the API surface. Documentation and examples are updated accordingly.
Score: 10/10
Clean implementation with proper validation, consistent error messages, and good test coverage.
Confidence Score: 2/5 - Changes NeededNot safe to merge — this PR introduces a critical silent behavioral regression in Key Findings:
Files requiring special attention
|
69a6a67 to
c32a964
Compare
There was a problem hiding this comment.
Overview: Consolidates containerTag and customId into a required options object for both TypeScript and Python OpenAI SDK wrappers, making conversation identity a first-class required parameter.
Issues found: None — this looks good to ship.
After reviewing the implementation:
- Validation is correct: Both TypeScript (
index.ts:74-96) and Python (middleware.py:695-707) properly validate required fields with type checks and.trim()/.strip()to reject whitespace-only strings - Defaults are correct:
addMemory = "never"(line 448 in middleware.ts, line 53 in middleware.py) andmode = "profile"— the previous automated reviews incorrectly claimed the default changed to"always", but the code clearly shows"never" - Test coverage is comprehensive: Unit tests verify validation, memory injection, and default behavior
- Breaking change is intentional: The API surface change from positional params to an options object with required
customIdis documented and improves consistency
Note: Several previous automated review comments flagged false positives about the addMemory default. The code at middleware.ts:448 is addMemory = "never", not "always".
Score: 10/10
Clean implementation with proper validation, consistent error messages across both SDKs, and solid test coverage.
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
Confidence Score: 4/5 - Mostly SafeLikely safe but review recommended — this is a well-scoped v2.0.0 breaking change that consistently renames Key Findings:
Files requiring special attention
|
c32a964 to
a73814b
Compare
There was a problem hiding this comment.
Overview: Makes customId/custom_id a required parameter in both TypeScript and Python OpenAI SDK wrappers, consolidating the API surface into a cleaner options-based interface.
Issues found: None — this looks good to ship.
After reviewing the implementation:
- Validation is correct: Both TypeScript (
index.ts:74-96) and Python (middleware.py:810-821) properly validate required fields with type checks and.trim()/.strip()to reject whitespace-only strings - Defaults are correct:
addMemory = "never"(line 448 in middleware.ts, line 53 in middleware.py) andmode = "profile"— contrary to several automated review comments, the default did NOT change to"always" - Test coverage is solid: Unit tests verify validation, memory injection, and default behavior for both SDKs
- Breaking change is intentional: The API change from optional
conversationIdto requiredcustomIdis by design and improves API consistency
Note: Many automated reviews flagged false positives about the addMemory default changing to "always". The code clearly shows addMemory = "never" in both SDKs.
Score: 10/10
Clean implementation with proper validation, consistent error messages across both SDKs, and comprehensive test coverage.

No description provided.