-
Notifications
You must be signed in to change notification settings - Fork 931
ConvexDB Integration #5883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
ConvexDB Integration #5883
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
@vishesh-baghel is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Major addition of ConvexDB as a new storage backend for Mastra, implementing comprehensive storage operations with real-time data subscriptions and reactive architecture.
- Implements complete storage interface in
stores/convex/src/storage/index.ts
with support for threads, messages, traces, evals, and workflow runs - Added real-time data subscriptions through Convex's reactive architecture in
stores/convex/convex/schema.ts
with proper indexing strategies - Includes Docker Compose setup in
stores/convex/docker-compose.yaml
for local development with health checks and AWS S3 integration - Comprehensive test coverage in
stores/convex/src/storage/index.test.ts
covering pagination, version handling, and data consistency - Warning: Auto-generated API utilities in
stores/convex/convex/_generated/api.js
use less type-safe 'anyApi', which should be addressed
23 files reviewed, 27 comments
Edit PR Review Bot Settings | Greptile
.env | ||
.env.local | ||
dist/ | ||
node_modules/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding .convex/ to exclude local Convex development files
volumes: | ||
- data:/convex/data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider naming volume more specifically like 'convex_data' to avoid conflicts
// Get all threads and delete them | ||
const threads = await ctx.db.query('threads').collect(); | ||
for (const thread of threads) { | ||
await ctx.db.delete(thread._id); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using Promise.all() for parallel deletion to improve performance when clearing tables
if (message.metadata !== undefined) { | ||
updateData.content = { | ||
...existingMessage.content, | ||
metadata: message.metadata, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Metadata update overwrites content.content field. Need to preserve both content and metadata separately.
if (message.metadata !== undefined) { | |
updateData.content = { | |
...existingMessage.content, | |
metadata: message.metadata, | |
}; | |
} | |
if (message.metadata !== undefined) { | |
updateData.content = { | |
...existingMessage.content, | |
content: existingMessage.content.content, | |
metadata: message.metadata, | |
}; | |
} |
// Save a message | ||
const message = await storage.saveMessage({ | ||
message: { | ||
id: 'message-789', | ||
threadId: 'thread-123', | ||
type: 'user', | ||
content: 'Hello, world!', | ||
createdAt: Date.now(), | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Example message schema in docs doesn't match the existing Mastra storage interface. Messages should use 'role' instead of 'type' and include an array of content objects with type and text fields.
* Save a trace | ||
*/ | ||
export const save = mutation({ | ||
args: { trace: v.any() }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Using v.any() for trace validation is too permissive. Should validate against the trace schema defined in schema.ts
// Check if trace already exists | ||
const existingTrace = await ctx.db | ||
.query('traces') | ||
.withIndex('by_traceId', q => q.eq('traceId', trace.id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Potential bug: using trace.id for traceId lookup when schema shows they're separate fields
// Create a separate query for counting total records | ||
// Need to rebuild the query with the same filters | ||
let countQuery; | ||
|
||
// Choose the appropriate index based on the provided filters | ||
if (args.traceId !== undefined) { | ||
const traceId = args.traceId; | ||
countQuery = ctx.db.query('traces').withIndex('by_traceId', q => q.eq('traceId', traceId)); | ||
} else if (args.parentSpanId !== undefined) { | ||
const parentSpanId = args.parentSpanId; | ||
countQuery = ctx.db.query('traces').withIndex('by_parentSpanId', q => q.eq('parentSpanId', parentSpanId)); | ||
} else { | ||
countQuery = ctx.db.query('traces'); | ||
} | ||
|
||
// Apply additional filters for count query | ||
if (args.traceId !== undefined && args.parentSpanId !== undefined) { | ||
countQuery = countQuery.filter(q => q.eq(q.field('parentSpanId'), args.parentSpanId)); | ||
} | ||
|
||
// Apply date range filters for count query | ||
if (args.startDate !== undefined) { | ||
const startDate = args.startDate; | ||
countQuery = countQuery.filter(q => q.gte(q.field('startTime'), startDate)); | ||
} | ||
|
||
if (args.endDate !== undefined) { | ||
const endDate = args.endDate; | ||
countQuery = countQuery.filter(q => q.lte(q.field('startTime'), endDate)); | ||
} | ||
|
||
// Get total count | ||
const total = await countQuery.collect().then(results => results.length); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Duplicate query construction logic between main query and count query. Extract to a shared function to avoid maintenance issues
// Return the paginated results with metadata | ||
// Structure the response to match the expected format with a 'traces' property | ||
return { | ||
page: Math.ceil(args.paginationOpts.numItems ? paginationResult.page.length / args.paginationOpts.numItems : 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Page calculation is incorrect when paginationOpts.numItems is undefined. This will result in incorrect pagination metadata
page: Math.ceil(args.paginationOpts.numItems ? paginationResult.page.length / args.paginationOpts.numItems : 1), | |
page: Math.ceil((skipCount + paginationResult.page.length) / numItems), |
{ | ||
"name": "@mastra/convex", | ||
"version": "0.2.0", | ||
"description": "Convex provider for Mastra - includes both vector and db storage capabilities", | ||
"type": "module", | ||
"main": "dist/index.js", | ||
"types": "dist/index.d.ts", | ||
"exports": { | ||
".": { | ||
"import": { | ||
"types": "./dist/index.d.ts", | ||
"default": "./dist/index.js" | ||
}, | ||
"require": { | ||
"types": "./dist/index.d.cts", | ||
"default": "./dist/index.cjs" | ||
} | ||
}, | ||
"./package.json": "./package.json" | ||
}, | ||
"scripts": { | ||
"build": "tsup src/index.ts --format esm,cjs --experimental-dts --clean --treeshake=smallest --splitting", | ||
"build:watch": "pnpm build --watch", | ||
"test": "vitest run", | ||
"test:watch": "vitest watch", | ||
"lint": "eslint ." | ||
}, | ||
"dependencies": { | ||
"convex": "1.25.2" | ||
}, | ||
"devDependencies": { | ||
"@internal/lint": "workspace:*", | ||
"@internal/storage-test-utils": "workspace:*", | ||
"@mastra/core": "workspace:^", | ||
"@microsoft/api-extractor": "^7.52.8", | ||
"@types/node": "^20.19.0", | ||
"eslint": "^9.29.0", | ||
"tsup": "^8.5.0", | ||
"typescript": "^5.8.3", | ||
"vitest": "^3.2.3" | ||
}, | ||
"peerDependencies": { | ||
"@mastra/core": ">=0.10.7-0 <0.11.0-0" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Missing LICENSE field which is present in pg provider. Add 'MIT' license field to match other packages
{ | |
"name": "@mastra/convex", | |
"version": "0.2.0", | |
"description": "Convex provider for Mastra - includes both vector and db storage capabilities", | |
"type": "module", | |
"main": "dist/index.js", | |
"types": "dist/index.d.ts", | |
"exports": { | |
".": { | |
"import": { | |
"types": "./dist/index.d.ts", | |
"default": "./dist/index.js" | |
}, | |
"require": { | |
"types": "./dist/index.d.cts", | |
"default": "./dist/index.cjs" | |
} | |
}, | |
"./package.json": "./package.json" | |
}, | |
"scripts": { | |
"build": "tsup src/index.ts --format esm,cjs --experimental-dts --clean --treeshake=smallest --splitting", | |
"build:watch": "pnpm build --watch", | |
"test": "vitest run", | |
"test:watch": "vitest watch", | |
"lint": "eslint ." | |
}, | |
"dependencies": { | |
"convex": "1.25.2" | |
}, | |
"devDependencies": { | |
"@internal/lint": "workspace:*", | |
"@internal/storage-test-utils": "workspace:*", | |
"@mastra/core": "workspace:^", | |
"@microsoft/api-extractor": "^7.52.8", | |
"@types/node": "^20.19.0", | |
"eslint": "^9.29.0", | |
"tsup": "^8.5.0", | |
"typescript": "^5.8.3", | |
"vitest": "^3.2.3" | |
}, | |
"peerDependencies": { | |
"@mastra/core": ">=0.10.7-0 <0.11.0-0" | |
} | |
} | |
{ | |
"name": "@mastra/convex", | |
"version": "0.2.0", | |
"description": "Convex provider for Mastra - includes both vector and db storage capabilities", | |
"license": "MIT", | |
"type": "module", | |
"main": "dist/index.js", | |
"types": "dist/index.d.ts", | |
"exports": { | |
".": { | |
"import": { | |
"types": "./dist/index.d.ts", | |
"default": "./dist/index.js" | |
}, | |
"require": { | |
"types": "./dist/index.d.cts", | |
"default": "./dist/index.cjs" | |
} | |
}, | |
"./package.json": "./package.json" | |
}, | |
"scripts": { | |
"build": "tsup src/index.ts --format esm,cjs --experimental-dts --clean --treeshake=smallest --splitting", | |
"build:watch": "pnpm build --watch", | |
"test": "vitest run", | |
"test:watch": "vitest watch", | |
"lint": "eslint ." | |
}, | |
"dependencies": { | |
"convex": "1.25.2" | |
}, | |
"devDependencies": { | |
"@internal/lint": "workspace:*", | |
"@internal/storage-test-utils": "workspace:*", | |
"@mastra/core": "workspace:^", | |
"@microsoft/api-extractor": "^7.52.8", | |
"@types/node": "^20.19.0", | |
"eslint": "^9.29.0", | |
"tsup": "^8.5.0", | |
"typescript": "^5.8.3", | |
"vitest": "^3.2.3" | |
}, | |
"peerDependencies": { | |
"@mastra/core": ">=0.10.7-0 <0.11.0-0" | |
} | |
} |
|
Description
This PR adds support for ConvexDB as a storage backend for Mastra, implementing the complete Mastra storage interface.
Key features implemented:
Closes #5344
Type of Change
Checklist