feat(angular-mcp-server): add violation grouping tool #23
Conversation
| const { join } = await import('node:path'); | ||
| const { basename } = await import('node:path'); |
There was a problem hiding this comment.
Couldn't that be actually imported at the of the file?
There was a problem hiding this comment.
Also, would be better const { join, basename } = await import('node:path'); ?
| const effectiveAfterPath = resolveCrossPlatformPath(cwd, contractAfterPath); | ||
|
|
||
| // Generate default save location if not provided | ||
| const { join, basename } = await import('node:path'); |
There was a problem hiding this comment.
Same here.
or it was done purposely to not load everything to memory and load it rather on demand?
|
|
||
| return `# ${group.name} | ||
| ## Summary |
AdrianRomanski
left a comment
AdrianRomanski
left a comment
There was a problem hiding this comment.
Overall it looks solid, but i dont like the "/" comments before initalization of variables.
They add nothing to the context more than method would do.
/** comments are good
| // Generate default save location if not provided | ||
| const { join } = await import('node:path'); | ||
| const { basename } = await import('node:path'); | ||
| const defaultSaveLocation = saveLocation |
There was a problem hiding this comment.
i will create method for readability, but that's up to you
| @@ -0,0 +1,243 @@ | |||
| import { createHandler } from '../shared/utils/handler-helpers.js'; | |||
There was a problem hiding this comment.
I suggest to delete comments as they are not giving more context than method names that are already there
| export function calculateComponentDistribution( | ||
| files: EnrichedFile[], | ||
| ): Record<string, number> { | ||
| const distribution: Record<string, number> = {}; |
There was a problem hiding this comment.
This could be done with reduce rather than nesting forEaches (functional vs side effect)
| export function convertComponentToFileFormat( | ||
| report: AllViolationsReport, | ||
| ): AllViolationsReportByFile { | ||
| const fileMap = new Map<string, FileViolationReport['components']>(); |
There was a problem hiding this comment.
This could be done with reduce rather than initalizing map, and then modifying it in forEach
| const defaultSaveLocation = saveLocation | ||
| ? saveLocation | ||
| : join( | ||
| 'tmp', |
There was a problem hiding this comment.
I guess this is a repetitive path piece tmp/.angular-toolkit-mcp, would be good to extract and move somewhere this default string
| ### Component Analysis | ||
|
|
||
| - **`report-violations`**: Report deprecated CSS usage in a directory with configurable grouping format | ||
| - **`report-violations`**: Report deprecated CSS usage for a specific DS component in a directory. Supports optional file output with statistics (components, files, lines). |
There was a problem hiding this comment.
The tool should be usage-agnostic as much as possible, the previous versions were more neutral and not DS specific
| 'Path where to save the diff result file. Supports both absolute and relative paths.', | ||
| 'Path where to save the diff result file. Supports both absolute and relative paths. If not provided, defaults to tmp/.angular-toolkit-mcp/contracts/diffs/<component-name>-diff.json', | ||
| }, | ||
| contractBeforePath: { |
There was a problem hiding this comment.
Contract diff has a default saveLocation path, would be good to have an unified approach to provide same default location for contractBefore-*/afterPath
| name: g.name, | ||
| rootPath, | ||
| directories: g.directories, | ||
| files: g.files.map((f) => ({ |
There was a problem hiding this comment.
Extract to a separate function
There was a problem hiding this comment.
Maybe utils should have their own types.ts file, because right now there are heaps of interfaces defined in multiple places, and a the end exported out of the utils.
This pull request introduces significant enhancements to the Angular MCP toolkit, specifically around reporting and organizing deprecated CSS violations. The main improvements include a new tool for distributing violations into balanced work groups, improved documentation, and better default file handling for component contract tools. These changes streamline large-scale migration workflows and make the toolkit easier to use and integrate.
New Violation Grouping Tool & Workflow Improvements
New tool for work distribution:
group-violationstool, which takes a saved violations report and organizes files into balanced work groups using a bin-packing algorithm. It ensures path exclusivity and maintains directory boundaries for parallel development, saving results as individual JSON and Markdown files. [1] [2] [3] [4]Documentation updates:
README.mdanddocs/tools.mdto describe the newgroup-violationstool, clarify the purpose and output ofreport-violationsandreport-all-violations, and revise recommended workflows to include group creation for parallel migration. [1] [2] [3] [4]Component Contract Tool Improvements
Default save location handling:
saveLocationto be optional in both the build and diff component contract tools. If not provided, a sensible default path is generated based on the component name, reducing required configuration and avoiding errors. [1] [2] [3] [4] [5] [6]Schema updates:
saveLocationis now optional and documented the new default path behavior. [1] [2] [3] [4]These changes make it easier to run large-scale migrations by automating the grouping of work, improving documentation, and simplifying file management for contract tools.