feat(data-mapper-v2): Make XSLT the source of truth for map definitions #8718
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change fundamentally shifts how Data Mapper v2 stores and loads maps: **Core Changes:** - XSLT is now the authoritative source for map definitions - Added XsltParser to extract map definitions from XSLT text value templates - Added XsltMetadataSerializer for v3 metadata format (UI layout only) - Removed LML embedding in XSLT comments (v2 format deprecated) **XPath to LML Conversion:** - Convert single-quoted string literals to double quotes for LML compatibility - Convert XPath infix math operators (+, -, *, div, mod) to function calls - Remove namespace prefixes (math:, xs:, etc.) from function names - Flatten associative operators (+, *) to multi-argument function calls **Bug Fixes:** - Fix delete button not working for custom values in unbounded inputs - Fix node position changes not triggering dirty state (Save button) **VS Code Extension:** - Update file service to use XSLT content directly - Add commands for loading/saving XSLT content
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
✅ Risk Level
✅ What & Why
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | Keep as-is. |
| Commit Type | ✅ | Correct (feature). |
| Risk Level | ✅ | Matches code changes (High). |
| What & Why | ✅ | Good; optionally add brief rollback/backups note. |
| Impact of Change | Add notes on CI/dev environment and extension packaging; mention backup during migration. | |
| Test Plan | ✅ | Unit tests present; add E2E/integration test or explanation why not. |
| Contributors | Optional: add contributors if applicable. | |
| Screenshots/Videos | Not required here. |
Final assessment: This PR passes the PR title/body validation and the advised risk level is risk:high, which matches your PR label.
Actionable recommendations before merge (high priority):
- Security & execution hardening
- The extension now calls execSync to run xslt3 (node) and writes temp files. Please:
- Prefer execFile / spawn with arg arrays to avoid shell injection or ensure inputs are strictly sanitized/quoted. The current command builds a single string; edge cases can lead to shell interpretation issues.
- Add unit/integration tests that simulate malformed or malicious XSLT content to ensure no shell injection or unexpected behavior.
- Confirm the
xslt3binary path resolution and fallback behavior (require.resolve) work in production packaging (VS Code extension packaging / hoisted node_modules). Add a clear error message and remediation steps in the UI if not found. - Document the required runtime and pre-conditions (e.g., which package versions are required, how the extension will ensure
xslt3is available in the production extension runtime).
- The extension now calls execSync to run xslt3 (node) and writes temp files. Please:
- Temp file handling & resource limits
- You added MAX_XSLT_SIZE and temp file cleanup — good. Please also consider:
- Using unique temp directories (you already use os.tmpdir + Date.now(); consider crypto.randomUUID for extra uniqueness in concurrent runs).
- Fail-safe cleanup on unexpected crashes and signal handling if needed.
- Unit tests for the cleanup path and for very-large XSLT failure mode.
- You added MAX_XSLT_SIZE and temp file cleanup — good. Please also consider:
- Dependency & licensing
- Pinpoint and document the new dependencies (saxon-js, xslt3) in the PR description with a short note on license and any packaging implications. CI maintainers will want to know these additions.
- Migration safety
- The migration strategy is present — ensure you add an explicit, user-facing hint that legacy files will be removed and how users can opt out or backup (e.g., create a local backup before migration, or create an automatic backup copy in a backups folder before deleting legacy LML files).
- Tests & CI
- Add at least one integration or E2E test that validates open/migrate/save cycle across the extension boundary, or add a short explanation in Test Plan and an issue to add it. This is especially important because the change adds many cross-layer interactions (webview ↔ extension host ↔ filesystem ↔ xslt3).
- Telemetry & error messages
- Ensure errors from xslt3 compilation are captured with enough telemetry (but no sensitive data). The code logs stderr; ensure you don't send large binary data into telemetry and that error text is user friendly.
If you address the above security/process items (exec/escape, packaging, backups/migration safety, and E2E/integration coverage or explicit justification), this large and impactful PR will be in good shape for final review and merge.
Thanks — this is a comprehensive, well-documented migration. Please reply with any follow-ups or if you want me to re-run a focused check on the test-plan vs files added.
Last updated: Thu, 22 Jan 2026 17:18:07 GMT
|
📊 Coverage check completed. See workflow run for details. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a major architectural change to make XSLT the source of truth for Data Mapper v2 definitions, replacing the previous LML (Logic Mapper Language) file format. The change consolidates three separate files (LML definition, XSLT transformation, and metadata JSON) into a single XSLT file with embedded metadata in an XML comment.
Changes:
- Introduces XSLT parsing and metadata serialization utilities to embed/extract map definitions and UI layout from XSLT files
- Implements local XSLT 3.0 transformation testing using SaxonJS, eliminating the macOS testing limitation
- Updates the save workflow to generate XSLT with embedded v3 metadata (layout only) where mapping logic is derived from XSLT content
- Adds backward compatibility for loading legacy LML files with automatic migration
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Adds build dependency for proper monorepo build ordering |
| pnpm-lock.yaml | Adds saxon-js and xslt3 dependencies for XSLT processing |
| libs/vscode-extension/src/lib/models/extensioncommand.ts | Adds commands for XSLT transform testing |
| libs/vscode-extension/src/lib/models/dataMapper.ts | Adds v3 format types and test result types |
| libs/data-mapper-v2/src/utils/index.ts | Exports new XSLT parser and serializer utilities |
| libs/data-mapper-v2/src/mapHandling/test/XsltParser.spec.ts | Comprehensive tests for XSLT parsing including Data Mapper backend format |
| libs/data-mapper-v2/src/mapHandling/test/XsltMetadataSerializer.spec.ts | Tests for metadata embedding/extraction and round-trip serialization |
| libs/data-mapper-v2/src/mapHandling/XsltParser.ts | Core XSLT parsing logic to derive map definitions from XSLT content |
| libs/data-mapper-v2/src/mapHandling/XsltMetadataSerializer.ts | Utilities for embedding/extracting metadata in XSLT comments |
| libs/data-mapper-v2/src/core/state/DataMapSlice.ts | Updates function position tracking and connection deletion logic |
| libs/data-mapper-v2/src/core/services/dataMapperFileService/dataMapperFileService.ts | Adds saveMapXsltCall method and deprecates legacy save methods |
| libs/data-mapper-v2/src/core/DataMapDataProvider.tsx | Handles XSLT transform test results from extension |
| libs/data-mapper-v2/src/components/test/TestPanel.tsx | Updates to use local XSLT transformation instead of backend API |
| libs/data-mapper-v2/src/components/commandBar/EditorCommandBar.tsx | Updates save workflow to generate and embed metadata in XSLT |
| libs/data-mapper-v2/src/components/codeView/CodeViewPanelBody.tsx | Changes code view to display XSLT instead of LML |
| docs/plans/2026-01-21-lml-to-xslt-migration-design.md | Design document explaining the migration strategy |
| apps/vs-code-react/src/webviewCommunication.tsx | Handles v3 format by parsing XSLT to derive map definitions |
| apps/vs-code-react/src/state/DataMapSliceV2.ts | Adds state for test transform results |
| apps/vs-code-react/src/run-service/types.ts | Defines message types for XSLT transform results |
| apps/vs-code-react/src/app/dataMapper/services/dataMapperFileService.ts | Implements saveMapXsltCall and testXsltTransform methods |
| apps/vs-code-react/src/app/dataMapper/appV2.tsx | Passes test results to DataMapDataProvider |
| apps/vs-code-designer/src/package.json | Adds saxon-js and xslt3 dependencies |
| apps/vs-code-designer/src/main.ts | Updates context to support both XSLT and legacy file extensions |
| apps/vs-code-designer/src/app/commands/dataMapper/extensionConfig.ts | Adds XSLT as primary file format and deprecates LML paths |
| apps/vs-code-designer/src/app/commands/dataMapper/dataMapper.ts | Implements loading from XSLT files with migration from LML |
| apps/vs-code-designer/src/app/commands/dataMapper/DataMapperPanel.ts | Adds handleTestXsltTransform for local XSLT transformation |
| apps/vs-code-designer/src/app/commands/dataMapper/DataMapperExt.ts | Adds metadata extraction utilities for v2/v3 formats |
| apps/vs-code-designer/package.json | Adds saxon-js and xslt3 dependencies to extension |
| Localize/lang/strings.json | Adds localization strings for XSLT-related messages |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
libs/data-mapper-v2/src/components/commandBar/EditorCommandBar.tsx:140
- The save workflow generates XSLT from the backend, embeds metadata, and saves it. However, if the backend XSLT generation fails (catch block at line 133), the metadata is never saved and the user loses their work. The earlier
saveDataMapdispatch (line 108) has already marked the map as saved in Redux state, creating an inconsistency. Consider rolling back the Redux state or showing a more prominent error if XSLT generation fails.
dispatch(
saveDataMap({
sourceSchemaExtended: sourceSchema,
targetSchemaExtended: targetSchema,
})
);
// Generate XSLT from the backend, then embed metadata and save
generateDataMapXslt(dataMapDefinition.definition)
.then((xsltStr) => {
// Embed metadata into the XSLT as a comment
const xsltWithMetadata = embedMetadataInXslt(xsltStr, xsltMetadata);
// Update XSLT content in state for code view display
dispatch(setXsltContent(xsltWithMetadata));
// Save the combined XSLT file (this is now the only file we need)
DataMapperFileService().saveMapXsltCall(xsltWithMetadata);
LoggerService().log({
level: LogEntryLevel.Verbose,
area: `${LogCategory.DataMapperDesigner}/onSaveClick`,
message: 'Successfully saved map with embedded metadata',
});
})
.catch((error: Error) => {
LoggerService().log({
level: LogEntryLevel.Error,
area: `${LogCategory.DataMapperDesigner}/onSaveClick`,
message: JSON.stringify(error.message),
});
DataMapperFileService().sendNotification(failedXsltMessage, error.message, LogEntryLevel.Error);
});
apps/vs-code-designer/src/app/commands/dataMapper/DataMapperPanel.ts
Outdated
Show resolved
Hide resolved
… robustness Security fixes: - Replace polynomial regex with string-based parsing in XsltMetadataSerializer - Add prototype pollution protection in XsltParser with safe property assignment Robustness improvements: - Add robust xslt3 path resolution checking multiple locations - Add XSLT size validation (5MB limit) before processing - Add XSLT parsing error handling with user-friendly messages - Handle empty map definition with null-coalescing fallback - Prevent stale test results with ref-based deduplication - Add detailed temp file cleanup logging Performance optimization: - Optimize position updates to avoid flooding undo history Testing: - Add comprehensive test suites for Redux slices, utilities, and query hooks - 217 new tests added (484 total tests now passing)
|
📊 Coverage check completed. See workflow run for details. |
|
📊 Coverage check completed. See workflow run for details. |
|
📊 Coverage check completed. See workflow run for details. |
1 similar comment
|
📊 Coverage check completed. See workflow run for details. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Type
Risk Level
What & Why
This change fundamentally shifts how Data Mapper v2 stores and loads maps by making XSLT the authoritative source for map definitions instead of LML files.
Why this change?
Impact of Change
Users:
Developers:
XsltParserextracts map definitions from XSLT text value templatesXsltMetadataSerializerfor v3 metadata format (UI layout embedded in XSLT)System:
Core Changes
XsltParser<xsl:text>and<xsl:value-of>templatesXsltMetadataSerializerDataMapperPanel.tsdataMapperFileService.tsXPath → LML Conversions
The parser handles several XPath-to-LML transformations:
'text'→"text")a + b→add(a, b))math:sqrt→sqrt)a + b + c→add(a, b, c))Bug Fixes Included
Test Plan
New Test Coverage
XsltParser.spec.ts- 446 lines covering XSLT parsing, XPath conversionXsltMetadataSerializer.spec.ts- 384 lines covering metadata embedding/extractionContributors
Screenshots/Videos
Design Document: See
docs/plans/2026-01-21-lml-to-xslt-migration-design.mdfor full architecture details.