Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #26 +/- ##
===========================================
- Coverage 51.13% 40.03% -11.10%
===========================================
Files 6 6
Lines 397 512 +115
Branches 63 63
===========================================
+ Hits 203 205 +2
- Misses 193 306 +113
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements version 1.6.0 of the source map parser MCP server, adding two new tools and improving the overall architecture. The main purpose is to expand functionality beyond just stack trace parsing to include general source map operations.
Key changes:
- Added
lookup_contextandunpack_sourcestools alongside the existingparse_stacktool - Implemented a declarative tool registration system with filtering capabilities
- Enhanced documentation and type definitions
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools.ts | Major refactoring to declarative tool system with new tool definitions and filtering logic |
| src/parser.ts | Added lookupContext and unpackSources methods to support new tools |
| src/index.ts | Exported additional ToolName type for external consumers |
| package.json | Version bump to 1.6.0 |
| README.md | Documentation updates for new tools with usage examples |
| README.zh-CN.md | Chinese documentation updates with reorganized content structure |
| function shouldRegisterTool(toolName: ToolName, filter?: ToolFilterOptions): boolean { | ||
| if (!filter) { | ||
| return true; | ||
| } | ||
|
|
||
| if (filter.allowList && filter.allowList.length > 0) { | ||
| return filter.allowList.includes(toolName); | ||
| } | ||
|
|
||
| if (filter.blockList && filter.blockList.length > 0) { | ||
| return !filter.blockList.includes(toolName); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
The filtering logic could be confusing when both allowList and blockList are provided. Consider adding validation to ensure only one filter type is used at a time, or document the precedence more clearly in the JSDoc.
|
|
||
| return result; | ||
| } catch (error) { | ||
| throw new Error("lookup context error: " + (error instanceof Error ? error.message : error), { |
There was a problem hiding this comment.
The error message 'lookup context error:' should be more descriptive. Consider using 'Failed to lookup source context:' to better indicate what operation failed.
| throw new Error("lookup context error: " + (error instanceof Error ? error.message : error), { | |
| throw new Error("Failed to lookup source context: " + (error instanceof Error ? error.message : error), { |
| totalSources: sourceMap.sources.length | ||
| }; | ||
| } catch (error) { | ||
| throw new Error("unpack sources error: " + (error instanceof Error ? error.message : error), { |
There was a problem hiding this comment.
The error message 'unpack sources error:' should be more descriptive. Consider using 'Failed to unpack source map sources:' to better indicate what operation failed.
| throw new Error("unpack sources error: " + (error instanceof Error ? error.message : error), { | |
| throw new Error("Failed to unpack source map sources: " + (error instanceof Error ? error.message : error), { |
No description provided.