-
Notifications
You must be signed in to change notification settings - Fork 17
Remove xmutil package and integrate Vensim conversion into engine2 #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The xmutil-js package contained a stale WASM build of Bob Eberlein's C++ xmutil tool for converting Vensim MDL to XMILE. This functionality is now provided natively by the Rust simlin-compat crate, exposed through engine2's Project.openVensim() API. Updated the example scripts and debug-diagram-gen to use openVensim() directly instead of the two-step convert-then-open pattern. Removed all workspace, tsconfig, and alias references. https://claude.ai/code/session_01HgDa6MrE8j46eW8AvLDanW
Pull Request Review: Remove xmutil package and integrate Vensim conversion into engine2SummaryThis PR successfully removes the Code Quality: ✅ ExcellentStrengths
Test Coverage: ✅ GoodThe Potential ImprovementsMinor: Error Handling ClarityIn the updated scripts ( inputFile.endsWith('.mdl')Consideration: This approach is simple and works well for the current use case. However, if you want to be more robust against edge cases (e.g., files named Minor: Logging LossThe old const [xmileContents, logs] = await convertMdlToXmile(contents, true);
if (logs) {
console.log('Conversion logs:', logs);
}The new
Question: Were the conversion logs primarily for debugging, or do users rely on them? Architecture & Design: ✅ Excellent
Performance: ✅ Neutral to Positive
Security: ✅ No ConcernsNo security issues identified. The change is a refactoring that doesn't introduce new attack surfaces. Documentation: ✅ Good
Compatibility NotesThis is a breaking change for any external code using
This is appropriate for the project's current state. Recommendations✅ Approve with minor observations This is a well-executed refactoring that simplifies the codebase without sacrificing functionality. The code quality is excellent, test coverage exists, and the documentation is updated appropriately. Optional Follow-ups (Non-blocking)
Great work on this cleanup! The new API is more intuitive and the codebase is simpler. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #218 +/- ##
=======================================
Coverage 76.82% 76.82%
=======================================
Files 69 69
Lines 20523 20523
=======================================
Hits 15766 15766
Misses 4757 4757 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
This PR removes the
@system-dynamics/xmutilpackage and integrates Vensim MDL file conversion directly into theengine2package. The conversion logic is now handled by a newEngine2Project.openVensim()method, simplifying the dependency tree and reducing code duplication.Key Changes
@system-dynamics/xmutilpackage: Deleted the entiresrc/xmutil-jsdirectory containing the WASM-based MDL to XMILE converterpackage.jsonworkspaces andtsconfig.jsonreferencesconvertMdlToXmile()calls withEngine2Project.openVensim()methodexamples/serialize.mjs,examples/svg.mjs, andscripts/debug-diagram-gen.mjsto use the new APIopenVensim()oropen()methodsCLAUDE.mdand website configuration filesImplementation Details
Engine2Project.openVensim()method handles Vensim MDL file conversion internallydebug-diagram-gen.mjsscript now usesengine2Project.toXmileString()to extract XMILE content, eliminating the need to track conversion statehttps://claude.ai/code/session_01HgDa6MrE8j46eW8AvLDanW