fix(mappings): carry OriginalValue and the snippet formatting through a rewrite - #383
Merged
Conversation
…gh a rewrite describe -> exec — how a document is copied — silently dropped a mapping's OriginalValue (the sample parsed from the JSON structure's snippet) and reformatted the structure's snippet from one line to multi-line. No build error either way; pure diff churn against a Studio Pro original. The old behaviour rested on mendixlabs#882, which measured TWO mappings a blank app ships and concluded Studio Pro always leaves OriginalValue empty. At corpus scale the opposite is more common: 2,322 of 3,042 value elements whose structure carries a sample store it. But neither global default is right, and measuring the SPLIT is what shows it. It is per DOCUMENT, not per element: 145 mappings carry the sample on EVERY element 107 carry it on NONE 2 are mixed So which one a mapping gets is a property of how and when it was authored, not something mxcli can derive. Always-copy is wrong for 107 mappings; always-empty is wrong for 145. A REWRITE does not have to choose — it knows what was stored, so it carries it (guard-don't-drop, ADR-0005), matching stored to rebuilt by JsonPath because names and order can change while the schema binding cannot. mendixlabs#882's actual decision is left intact: a NEWLY authored mapping still writes empty. Its comment is corrected in place rather than deleted, since the decision stands and only its measurement was too narrow. The export side needed the field added to the semantic type and both readers — its codec writer hardcoded "" rather than carrying it at all. The snippet half is the same shape: keep the stored formatting when the JSON is semantically equal, comparing decoded values rather than strings. Anything that does not parse counts as different, so a malformed snippet is replaced rather than silently kept. Verified against ako/TestApp's hand-authored mappings: all three documents are now IDENTICAL after describe -> exec, and the executor reports Unchanged. Controls: reverting the carry reports Modified and loses the samples; reverting the JSON comparison fails the formatting cases. Closes #379 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Closes #379.
describe→exec— how a document is copied — silently dropped a mapping'sOriginalValueand reformatted the JSON structure's snippet. No build erroreither way; pure diff churn against a Studio Pro original.
Found by round-tripping ako/TestApp's
Mappingsmodule, hand-authored in StudioPro rather than shipped in a marketplace module. All three documents are now
identical after
describe→exec, and the executor reportsUnchanged.Neither default was right, and the split is what shows it
The old behaviour rested on mendixlabs#882, which measured two mappings a blank app
ships and concluded Studio Pro always leaves
OriginalValueempty. At corpusscale the opposite is more common — 2,322 of 3,042 value elements whose
structure carries a sample store it.
But "always copy" is not the fix either. The split is per document:
So which one a mapping gets is a property of how and when it was authored, not
something mxcli can derive. Always-copy is wrong for 107 mappings; always-empty
is wrong for 145.
A rewrite does not have to choose. It knows what was stored, so it carries it
forward — guard-don't-drop, ADR-0005 —
matching stored to rebuilt by
JsonPath, because names and order can changewhile the schema binding cannot.
mendixlabs#882's actual decision is left intact: a newly authored mapping still writes
empty. Its comment is corrected in place rather than deleted, since the decision
stands and only its measurement was too narrow.
The export side needed
OriginalValueadded to the semantic type and to bothreaders — its codec writer hardcoded
""rather than carrying the field at all.The snippet half
describepretty-prints, soexecstored the pretty form and a snippet StudioPro had on one line came back multi-line. Same JSON, different bytes. The
rewrite now keeps the stored formatting when the JSON is semantically equal,
comparing decoded values rather than strings. Anything that does not parse
counts as different, so a malformed snippet is replaced rather than silently
kept.
Verification
ItemImportMapping,Export_mappinganditem_jsonallIDENTICAL after
describe→exec; executor reportsUnchangedfor allthree.
Modifiedand loses the samples;reverting the JSON comparison fails the formatting and key-order cases.
empty) and the
JsonPath-matching rule.Full suite,
go vet,gofmtandmake check-mdl(462 PASS) are green.A note for the next person
The general shape is worth keeping: when a measurement says "always X" from a
small sample and a wider one says "sometimes X", check whether the split is
per-document before picking a default. A per-document split usually means the
answer is "preserve", not "choose". Recorded in the finding.
🤖 Generated with Claude Code