gremlin-mcp #3238
gremlin-mcp #3238
Conversation
gremlin-mcp/src/main/javascript/src/gremlin/models/graph-schema.ts
Outdated
Show resolved
Hide resolved
| valid queries, generating meaningful filters, and explaining results in natural language. For operators, it offers safer | ||
| and more efficient interactions by avoiding blind exploratory scans, enabling caching and change detection, and | ||
| providing hooks to steer what should or shouldn’t be surfaced (for example, excluding sensitive or non‑categorical | ||
| fields). In short, schema discovery turns an opaque dataset into an actionable contract between your graph and the tools |
There was a problem hiding this comment.
excluding sensitive or non‑categorical fields
How does it make that determination? It might be worth adding examples, such as id.
| * `GREMLIN_ENUM_PROPERTY_DENYLIST` — comma-separated property names to exclude from enum detection (default: `id,pk,name,description,startDate,endDate,timestamp,createdAt,updatedAt`) | ||
| * `GREMLIN_SCHEMA_MAX_ENUM_VALUES` — limit the number of enum values returned per property in the schema (default: `10`) | ||
| * `GREMLIN_SCHEMA_INCLUDE_SAMPLE_VALUES` — include small example values for properties in the schema (default: `false`) | ||
| * `GREMLIN_SCHEMA_INCLUDE_COUNTS` — include approximate vertex/edge label counts in the schema (default: `true`) |
There was a problem hiding this comment.
This may be the costliest query to run and potentially the least useful to an LLM. Perhaps we should default this to false?
Alternatively, we could make the counts a separate MCP tool exposed to the agents.
There was a problem hiding this comment.
making this simple change, kicked open a wide world of issues i had to investigate. fixed logging and configuration setup as part of making that change.
gremlin-javascript/src/main/javascript/gremlin-javascript/package.json
Outdated
Show resolved
Hide resolved
| "verbatimModuleSyntax": true | ||
| }, | ||
| "include": ["src/**/*"], | ||
| "exclude": ["node_modules", "dist", "tests"], |
There was a problem hiding this comment.
With the tests being excluded from the TypeScript config, they are not compiled in to the dist/ folder (good) but also don't get any of the tsconfig settings above (bad). In Kiro (and I suspect VSCode) I ran in to a lot of typing issues in the tests. Mostly with Jest types that should be global, but aren't. Also, the Node process instance could not be found, leading to type errors in the editor.
The only solution I can think of is not a great one. You would need to have two separate tsconfig files, one for building the distribution artifacts and one for the tests that doesn't emit. Then your build script in the package.json would need to target the correct build tsconfig file. It's more complicated, but should resolve the issue.
It's worth noting that running npm test works perfectly. It's just in the editor that I ran in to issues.
There was a problem hiding this comment.
eh, intellij is not super happy either so perhaps this is the reason why? not sure i know what to do with this exactly even with your suggestion. might need to talk about it after this merges.
| const result = await Effect.runPromiseExit(generateEdgePatterns(mockTraversalSource)); | ||
|
|
||
| expect(result._tag).toBe('Failure'); |
There was a problem hiding this comment.
| const result = await Effect.runPromiseExit(generateEdgePatterns(mockTraversalSource)); | |
| expect(result._tag).toBe('Failure'); | |
| await expect(() => Effect.runPromise(generateEdgePatterns(mockTraversalSource))).rejects.toEqual(error); |
| const result = await Effect.runPromiseExit( | ||
| analyzeSingleProperty(mockTraversalSource, 'person', 'name', mockConfig, true) | ||
| ); | ||
|
|
||
| expect(result._tag).toBe('Failure'); |
There was a problem hiding this comment.
| const result = await Effect.runPromiseExit( | |
| analyzeSingleProperty(mockTraversalSource, 'person', 'name', mockConfig, true) | |
| ); | |
| expect(result._tag).toBe('Failure'); | |
| await expect(() => Effect.runPromise( | |
| analyzeSingleProperty(mockTraversalSource, 'person', 'name', mockConfig, true) | |
| )).rejects.toEqual(error); |
| const result = await Effect.runPromiseExit( | ||
| executeGremlinQuery(mockQuery, 'Test query failed', 'test query') | ||
| ); | ||
|
|
||
| expect(result._tag).toBe('Failure'); |
There was a problem hiding this comment.
| const result = await Effect.runPromiseExit( | |
| executeGremlinQuery(mockQuery, 'Test query failed', 'test query') | |
| ); | |
| expect(result._tag).toBe('Failure'); | |
| await expect(() => Effect.runPromise(processBatched(items, 2, processor))).rejects.toThrow(); |
| const result = await Effect.runPromiseExit(validateVertices(invalidVertices)); | ||
|
|
||
| expect(result._tag).toBe('Failure'); |
There was a problem hiding this comment.
| const result = await Effect.runPromiseExit(validateVertices(invalidVertices)); | |
| expect(result._tag).toBe('Failure'); | |
| await expect(() => | |
| Effect.runPromise(validateVertices(invalidVertices)) | |
| ).rejects.toBeDefined(); |
This might be a nit pick and subjective, but testing an internal _tag property for a particular string seems like a smell.
I much prefer leaning on typical Promise behavior and Jest Promise support, which avoids the extra Exit wrapping and makes these tests a bit more consistent with the success tests.
There are many tests in this file that fit this pattern, but I had a hell of time getting the code suggestion stuff working, so I'm going to give you this one example.
It would be even nicer if these tests validated which error is thrown.
spmallette
force-pushed
the
gremlin-mcp
branch
from
2306a08 to
fc03a4f
Compare
Docs about unexpected mutations by gremlin-mcp
We could bring this back later. Currently feels a little tricky to use in testing and release of 3.8.0 is looming.
Allows for the basic plain text authentication with Gremlin Server.
The export tool isn't working consistently enough for release. Can bring it back later.
Fixed problems with that setting - it was not having any effect. Also fixed logging to stderr setup for Effect.
spmallette
force-pushed
the
gremlin-mcp
branch
from
fc03a4f to
6e41d5b
Compare
|
VOTE +1 (non-binding) |
4 participants
After #3196 was offered, it was pulled to the "gremlin-mcp" branch for refinements.
Once merged, this is likely ready for 3.8.0.alpha1 release in preparation for the official release with the actual 3.8.0.
VOTE +1