Conversation
- Enhanced .env.example with detailed PgVector HNSW index configuration options. - Removed deprecated GitHub Copilot integration code from copilot.ts. - Updated pg-storage.ts to reflect changes in PgVector dimensions and improved logging for PostgreSQL Store initialization. - Changed API route path from "/api/researchAgent" to "/custom/researchAgent" in index.ts.
|
Keep this PR in a mergeable state → Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates PgVector-related configuration and logging, makes IVF list counts configurable via environment variables, adjusts dimensions to 3072 for gemini-embedding-001, removes deprecated Copilot integration, and changes the research agent API route path. Class diagram for updated PgVector and memory configurationclassDiagram
class PgVector {
+string connectionString
+string tenantId
+string tableName
+string column
+string idColumn
+string metadataColumn
+string dimensions
+string indexName
+init()
}
class PgVectorIndexConfig {
+string type
+string metric
+PgVectorIvfConfig ivf
}
class PgVectorIvfConfig {
+number lists
%% now configurable via env LISTS, default 3072
}
class Memory {
+MemoryIndexConfig longTermMemory
+MemoryIndexConfig workingMemory
}
class MemoryIndexConfig {
+string name
+PgVectorIndexConfig indexConfig
}
class GraphRAGTool {
+string vectorStoreName
+string indexName
+string model
+GraphOptions graphOptions
}
class GraphOptions {
+number dimension
%% 3072 for gemini_embedding_001
+number threshold
+number randomWalkSteps
+number restartProb
+number topK
}
class EnvironmentConfig {
+string SUPABASE
+string MASTRA_PG_CONNECTION_STRING
+string TENANT_ID
+string LISTS
%% used to compute ivf.lists
+string GRAPH_THRESHOLD
+string GRAPH_RANDOM_WALK_STEPS
+string GRAPH_RESTART_PROB
+string GRAPH_TOP_K
}
PgVector "1" --> "1" PgVectorIndexConfig : uses
PgVectorIndexConfig "1" --> "1" PgVectorIvfConfig : has
Memory "1" --> "2" MemoryIndexConfig : configures
MemoryIndexConfig "1" --> "1" PgVectorIndexConfig : uses
GraphRAGTool "1" --> "1" GraphOptions : configured_with
EnvironmentConfig ..> PgVectorIvfConfig : configures_lists
EnvironmentConfig ..> GraphOptions : configures_thresholds
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
🤖 Hi @ssdeanx, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
|
Caution Review failedThe pull request is closed. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughConfiguration updates to vector embeddings and RAG parameters (SEMANTIC_TOP_K, PG_MIN_SCORE, PG_EF, dimension 1568→3072), introduction of IVF list and HNSW parameters, complete removal of GitHub Copilot integration module, and a researchAgent API route path adjustment from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ssdeanx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on refining the application's configuration for vector embeddings and cleaning up deprecated code. It introduces more granular control over PgVector HNSW index settings, ensuring better performance and flexibility for high-dimensional embeddings. Additionally, it removes an outdated GitHub Copilot integration and adjusts an API route for improved consistency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Instead of using
console.log(allIndexes)with an ESLint disable, consider routing this through the existinglogutility or behind a debug flag so it can be controlled per environment and avoids bypassing lint rules. - The
parseInt(process.env.LISTS ?? '3072')calls would be safer and clearer with an explicit radix (e.g.,parseInt(..., 10)) or by usingNumber()plus a NaN fallback to ensure invalid LISTS values don't silently propagate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of using `console.log(allIndexes)` with an ESLint disable, consider routing this through the existing `log` utility or behind a debug flag so it can be controlled per environment and avoids bypassing lint rules.
- The `parseInt(process.env.LISTS ?? '3072')` calls would be safer and clearer with an explicit radix (e.g., `parseInt(..., 10)`) or by using `Number()` plus a NaN fallback to ensure invalid LISTS values don't silently propagate.
## Individual Comments
### Comment 1
<location> `src/mastra/config/pg-storage.ts:109-112` </location>
<code_context>
type: 'flat', // flat index type (supports dimensions > 4000, unlike HNSW limit of 2000)
metric: 'cosine', // Distance metric for normalized embeddings
- ivf: {lists: 4000},
+ ivf: {lists: parseInt(process.env.LISTS ?? '3072')}, // IVF configuration for flat index
}
},
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle invalid LISTS env values and specify a radix for parseInt to avoid subtle bugs.
If `process.env.LISTS` is set to a non-integer, `parseInt` will return `NaN`, which may propagate into the IVF config and cause runtime errors. Please (1) pass an explicit radix (e.g., `parseInt(..., 10)`) and (2) fall back to the default (3072) when parsing fails, e.g. via a small helper that validates and normalizes this value once.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ivf: {lists: parseInt(process.env.LISTS ?? '3072')}, // IVF configuration for flat index | ||
| } | ||
| }, | ||
| // Enhanced working memory with supported template |
There was a problem hiding this comment.
issue (bug_risk): Handle invalid LISTS env values and specify a radix for parseInt to avoid subtle bugs.
If process.env.LISTS is set to a non-integer, parseInt will return NaN, which may propagate into the IVF config and cause runtime errors. Please (1) pass an explicit radix (e.g., parseInt(..., 10)) and (2) fall back to the default (3072) when parsing fails, e.g. via a small helper that validates and normalizes this value once.
|
🤖 I'm sorry @ssdeanx, but I was unable to process your request. Please see the logs for more details. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates configuration for PgVector, cleans up deprecated code, and adjusts an API route. The new configurations in .env.example are a good addition, but the comments could be improved for clarity and formatting. The changes in pg-storage.ts correctly reflect the new embedding dimensions and make the setup more flexible. The removal of the Copilot integration and the API route change are straightforward. I've added a few comments to suggest improvements to the configuration file and to clean up a leftover debug log.
| LISTS='3072' # IVF lists for PgVector (adjust based on dataset size, default: 3072 for gemini-embedding-001 1-4k limits, this so can use 3072 dims) | ||
| #PG_HNSW_M=16 # HNSW connections per layer (default: 16, higher = better recall, more memory) | ||
| #PG_HNSW_EF_CONSTRUCTION=64 # HNSW build-time candidates (default: 64, higher = better quality, slower build) | ||
| PG_EF='79' # HNSW query-time candidates (default: 100, higher = better recall, slower queries) | ||
| PG_MIN_SCORE='0.65' # Minimum similarity score for vector search results (default: 0.7, lower = more results, less relevant) | ||
| # Graph-based Retrieval Configuration | ||
| GRAPH_THRESHOLD='0.7' # Graph similarity threshold for random walk connections (0.0 - 1.0 range high to low) | ||
| GRAPH_RANDOM_WALK_STEPS='10' # Number of random walk steps for graph-based retrieval (default: 10, higher = more exploration, slower) | ||
| SEMANTIC_TOP_K='4' # Number of top semantic results to retrieve (default: 5 higher = more results, slower) |
There was a problem hiding this comment.
The comments in this new configuration block have some formatting and clarity issues that affect readability:
- Inconsistent Alignment: The inline comments are not aligned, making the file harder to scan.
- Unclear
LISTScomment (line 53): The phrasethis so can use 3072 dimsis grammatically incorrect and unclear. - Ambiguous
GRAPH_THRESHOLDcomment (line 59): The range(0.0 - 1.0 range high to low)is ambiguous. It should be clarified whether a high value means high similarity.
Improving these will make the configuration easier to understand for other developers. For example, the comment for GRAPH_THRESHOLD could be changed to # Graph similarity threshold for random walk connections (0.0 to 1.0). Higher values mean stricter connections.
| // eslint-disable-next-line no-console | ||
| console.log(allIndexes); |
There was a problem hiding this comment.
This console.log appears to be for debugging, especially since a log.info call with the same information follows. It's best practice to use the established logger for all output and remove temporary console.log statements before merging. The eslint-disable comment suggests this was a temporary addition that should now be cleaned up.
Greptile OverviewGreptile SummaryThis PR updates PgVector configuration to support 3072-dimension embeddings for Key Changes:
Considerations:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client as API Client
participant Route as Mastra Route Handler
participant Agent as Research Agent
participant Memory as PgMemory
participant Vector as PgVector (3072D)
participant DB as PostgreSQL
Note over Client,Route: Breaking Change: /api/researchAgent → /custom/researchAgent
Client->>Route: POST /custom/researchAgent
Route->>Agent: Execute research task
Agent->>Memory: Retrieve semantic context
Memory->>Vector: Query with topK=4
Vector->>DB: HNSW search (EF=79, LISTS=3072)
DB-->>Vector: Return vectors (3072 dims)
Vector-->>Memory: Return results (min_score=0.65)
Memory-->>Agent: Context retrieved
Agent->>Memory: Store new embeddings
Memory->>Vector: Upsert vectors
Vector->>DB: Store with IVF index
DB-->>Vector: Success
Vector-->>Memory: Success
Memory-->>Agent: Success
Agent-->>Route: Response with context
Route-->>Client: JSON response
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # PgVector HNSW Index Configuration (for high-dimensional embeddings) | ||
| # HNSW index type supports dimensions > 2000 (IVFFlat is limited to 2000) | ||
| LISTS='3072' # IVF lists for PgVector (adjust based on dataset size, default: 3072 for gemini-embedding-001 1-4k limits, this so can use 3072 dims) |
There was a problem hiding this comment.
style: Comment says "1-4k limits, this so can use 3072 dims" - wording is unclear. Consider: "adjust based on dataset size, default: 3072 to match gemini-embedding-001 dimension range (1-4k supported)"
Prompt To Fix With AI
This is a comment left during a code review.
Path: .env.example
Line: 53:53
Comment:
**style:** Comment says "1-4k limits, this so can use 3072 dims" - wording is unclear. Consider: "adjust based on dataset size, default: 3072 to match gemini-embedding-001 dimension range (1-4k supported)"
How can I resolve this? If you propose a fix, please make it concise.| }), | ||
| chatRoute({ | ||
| path: "/api/researchAgent", | ||
| path: "/custom/researchAgent", |
There was a problem hiding this comment.
style: API route changed from /api/researchAgent to /custom/researchAgent - breaking change for clients.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mastra/index.ts
Line: 316:316
Comment:
**style:** API route changed from `/api/researchAgent` to `/custom/researchAgent` - breaking change for clients.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Pull request overview
This pull request updates the PgVector configuration and cleans up deprecated code. The main changes focus on updating embedding dimensions from 1568 to 3072 and adjusting IVF configuration parameters, removing unused GitHub Copilot integration code, and changing a custom API route path. However, there are several documentation inconsistencies that need attention, particularly around index type descriptions and embedding model specifications.
Key Changes
- PgVector configuration updates: Changed embedding dimensions from 1568 to 3072, with corresponding updates to IVF list counts and configuration comments
- Deprecated code removal: Removed GitHub Copilot integration from copilot.ts (no remaining references in codebase)
- API route restructure: Changed research agent route from
/api/researchAgentto/custom/researchAgent
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| .env.example | Added detailed PgVector configuration documentation with IVF and HNSW parameters, though contains several documentation inaccuracies about index types |
| src/mastra/config/copilot.ts | Completely removed deprecated GitHub Copilot OpenAI compatible integration code |
| src/mastra/config/pg-storage.ts | Updated dimension references from 1568 to 3072, adjusted IVF lists configuration to use environment variable, updated related comments |
| src/mastra/index.ts | Changed research agent chat route path from /api/ to /custom/ prefix for better organization |
| indexName: 'memory_messages_3072', | ||
| model: google.textEmbedding('gemini-embedding-001'), | ||
| // Supported graph options (updated for 1568 dimensions) | ||
| // Supported graph options (updated for 3072 dimensions) |
There was a problem hiding this comment.
The comment was updated from "1568 dimensions" to "3072 dimensions" but this is inconsistent with the actual capabilities of gemini-embedding-001, which produces 768-dimensional embeddings by default. The dimension value should match the actual model being used. Please verify the model and dimension configuration are aligned.
| // Supported graph options (updated for 3072 dimensions) | ||
| graphOptions: { | ||
| dimension: 3072, // gemini-embedding-001 dimension (1568) | ||
| dimension: 3072, // gemini-embedding-001 dimension (3072) |
There was a problem hiding this comment.
The inline comment "(3072)" duplicates information from the preceding comment on line 178. Since line 178 already states "updated for 3072 dimensions", this inline repetition is redundant and should be removed for cleaner code.
| dimension: 3072, // gemini-embedding-001 dimension (3072) | |
| dimension: 3072, // gemini-embedding-001 dimension |
|
|
||
| # PgVector HNSW Index Configuration (for high-dimensional embeddings) | ||
| # HNSW index type supports dimensions > 2000 (IVFFlat is limited to 2000) | ||
| LISTS='3072' # IVF lists for PgVector (adjust based on dataset size, default: 3072 for gemini-embedding-001 1-4k limits, this so can use 3072 dims) |
There was a problem hiding this comment.
This comment is confusing and unclear. The phrase "default: 3072 for gemini-embedding-001 1-4k limits, this so can use 3072 dims" is grammatically incorrect and difficult to parse. Consider revising to something clearer like: "IVF lists for flat index (default: 3072, adjust based on dataset size. Higher values work better for larger datasets)"
| LISTS='3072' # IVF lists for PgVector (adjust based on dataset size, default: 3072 for gemini-embedding-001 1-4k limits, this so can use 3072 dims) | |
| LISTS='3072' # Number of IVF lists for PgVector (default: 3072). Adjust based on dataset size; higher values work better for larger datasets and high-dimensional embeddings. |
| @@ -105,7 +106,7 @@ export const pgMemory = new Memory({ | |||
| indexConfig: { | |||
| type: 'flat', // flat index type (supports dimensions > 4000, unlike HNSW limit of 2000) | |||
There was a problem hiding this comment.
The comment states "flat index type (supports dimensions > 4000, unlike HNSW limit of 2000)" but this is misleading in the context of the actual configuration. The flat index is being configured with IVF (see line 109), which is typically called IVFFlat in pgvector documentation. While technically accurate that flat-based indexes support higher dimensions than HNSW, the comment could be clearer. Consider: "flat index type with IVF (supports dimensions > 2000, unlike HNSW which is limited to 2000)"
| type: 'flat', // flat index type (supports dimensions > 4000, unlike HNSW limit of 2000) | |
| type: 'flat', // flat index type with IVF (supports dimensions > 2000, unlike HNSW which is limited to 2000) |
| #PG_HNSW_M=16 # HNSW connections per layer (default: 16, higher = better recall, more memory) | ||
| #PG_HNSW_EF_CONSTRUCTION=64 # HNSW build-time candidates (default: 64, higher = better quality, slower build) |
There was a problem hiding this comment.
These commented-out HNSW configuration parameters (PG_HNSW_M and PG_HNSW_EF_CONSTRUCTION) are misleading in this context. The actual implementation in pg-storage.ts uses a 'flat' index type with IVF, not HNSW, because HNSW is limited to 2000 dimensions. Since the embedding dimension is 3072, these HNSW-specific parameters are not applicable and should either be removed or moved to a different section with a clear note that they're only for HNSW indexes with dimensions ≤ 2000.
| #PG_HNSW_M=16 # HNSW connections per layer (default: 16, higher = better recall, more memory) | |
| #PG_HNSW_EF_CONSTRUCTION=64 # HNSW build-time candidates (default: 64, higher = better quality, slower build) |
| PG_EF='79' # HNSW query-time candidates (default: 100, higher = better recall, slower queries) | ||
| PG_MIN_SCORE='0.65' # Minimum similarity score for vector search results (default: 0.7, lower = more results, less relevant) | ||
| # Graph-based Retrieval Configuration | ||
| GRAPH_THRESHOLD='0.7' # Graph similarity threshold for random walk connections (0.0 - 1.0 range high to low) |
There was a problem hiding this comment.
The phrase "0.0 - 1.0 range high to low" is grammatically unclear. It should be clarified to indicate what high and low values mean, such as "0.0 - 1.0 range (higher = stricter)" or "0.0 - 1.0 range (lower = more connections)".
| GRAPH_THRESHOLD='0.7' # Graph similarity threshold for random walk connections (0.0 - 1.0 range high to low) | |
| GRAPH_THRESHOLD='0.7' # Graph similarity threshold for random walk connections (0.0 - 1.0 range, higher = stricter, fewer connections) |
| # Graph-based Retrieval Configuration | ||
| GRAPH_THRESHOLD='0.7' # Graph similarity threshold for random walk connections (0.0 - 1.0 range high to low) | ||
| GRAPH_RANDOM_WALK_STEPS='10' # Number of random walk steps for graph-based retrieval (default: 10, higher = more exploration, slower) | ||
| SEMANTIC_TOP_K='4' # Number of top semantic results to retrieve (default: 5 higher = more results, slower) |
There was a problem hiding this comment.
Missing comma after "default: 5" for consistency with other comments. Should be "(default: 5, higher = more results, slower)" to match the style of other configuration comments in this file.
| SEMANTIC_TOP_K='4' # Number of top semantic results to retrieve (default: 5 higher = more results, slower) | |
| SEMANTIC_TOP_K='4' # Number of top semantic results to retrieve (default: 5, higher = more results, slower) |
| # PgVector HNSW Index Configuration (for high-dimensional embeddings) | ||
| # HNSW index type supports dimensions > 2000 (IVFFlat is limited to 2000) |
There was a problem hiding this comment.
This comment is misleading. The statement "HNSW index type supports dimensions > 2000 (IVFFlat is limited to 2000)" is incorrect - it's actually the opposite. According to pgvector documentation, HNSW is limited to 2000 dimensions, while IVFFlat (and flat/IVF) can support higher dimensions. The actual code in pg-storage.ts correctly uses 'flat' index type (line 107), not HNSW. Consider updating this comment to: "# PgVector Flat/IVF Index Configuration (for high-dimensional embeddings)" and clarifying that IVF/Flat is used because HNSW is limited to 2000 dimensions.
| # PgVector HNSW Index Configuration (for high-dimensional embeddings) | |
| # HNSW index type supports dimensions > 2000 (IVFFlat is limited to 2000) | |
| # PgVector Flat/IVF Index Configuration (for high-dimensional embeddings) | |
| # IVF/Flat index types support dimensions > 2000 (HNSW is limited to 2000). Use IVF/Flat for high-dimensional embeddings. |
| type: 'flat', // flat index type (supports dimensions > 4000, unlike HNSW limit of 2000) | ||
| metric: 'cosine', // Distance metric for normalized embeddings | ||
| ivf: {lists: 4000}, | ||
| ivf: {lists: parseInt(process.env.LISTS ?? '3072')}, // IVF configuration for flat index |
There was a problem hiding this comment.
The comment "IVF configuration for flat index" is somewhat redundant since the code itself clearly shows this is IVF configuration. Consider making it more descriptive about what the 'lists' parameter does, e.g., "Number of IVF clusters (lists) for partitioning the index" or simply remove the comment if it doesn't add value beyond what the code shows.
| ivf: {lists: parseInt(process.env.LISTS ?? '3072')}, // IVF configuration for flat index | |
| ivf: {lists: parseInt(process.env.LISTS ?? '3072')}, // Number of IVF clusters (lists) for partitioning the index |
| }); | ||
|
|
||
| // PgVector configuration for 1568 dimension embeddings (gemini-embedding-002) | ||
| // PgVector configuration for 3072 dimension embeddings (gemini-embedding-001) |
There was a problem hiding this comment.
The comment states "gemini-embedding-001" which produces 768-dimensional embeddings by default, not 3072. If you're actually using 3072-dimensional embeddings, you're likely using a different model like "text-embedding-004" or have configured gemini-embedding-001 with a different output dimension. Please verify and update the comment to reflect the actual model being used.
| // PgVector configuration for 3072 dimension embeddings (gemini-embedding-001) | |
| // PgVector configuration for 768 dimension embeddings (gemini-embedding-001 by default) |
1 participant
Summary by Sourcery
Align PostgreSQL vector storage and routing configuration with updated PgVector and embedding model settings while removing obsolete Copilot integration.
New Features:
Enhancements:
Documentation:
Chores: