fix(migrate): write no nonunique where the v2 usage must be unique; messages demo validates clean - #795
Merged
Conversation
A message in a part implicitly subsets the unique Parts::Part::ownedActions, so the two channel messages cannot be nonunique. The example validates clean, leaves the known-failure list (which held only it), and the pilot-differential baseline re-records the examples digest. Fixes #758 Co-Authored-By: jason.han <hanhuijun@gmail.com>
A v1 property with isUnique=false, or a MagicDraw [] / [n] type modifier, that becomes a usage implicitly subsetting a unique library feature, or that redefines or subsets a feature written unique, is written without nonunique; the report notes the dropped modifier and marks the entry approximated. The decision reuses the checker's implicit-subsetting rules: semantics now exposes ImplicitSubsettingCandidates over declaration kinds alone, and the migrator reads the candidates' uniqueness from the bundled library. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
…iqueness slotConflict read the v1 isUnique flag, so a repeated instance in a slot of a nonunique composite property passed although the part is now written unique. It asks featureWrittenUnique instead, which also admits a repeat on a feature an array type modifier writes nonunique. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Merged
3 of 6 tasks
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.
What and why
Fixes #758
Follows #748 (
fix(check): conformance of an implicit subsetting), which is ondevelopalready (5fb9a8434); this branch is cut from it, so no landing-order constraint remains. #748 madesubsetting-uniqueness-conformancehold of the feature a usage implicitly subsets, which exposed two things this PR corrects:1.
examples/parser_features_demo_messages_events.sysml(fe28318f3). The two channel messages were declaredmessage incoming: Message[*] nonunique;/message outgoing: Message[*] nonunique;; a message in a part implicitly subsets the uniqueParts::Part::ownedActions, so the modifier is dropped and nothing else in the demo changes (no prose in the demo orexamples/PARSER_FEATURES_DEMOS.mddescribed the messages as nonunique). TheexamplesKnownFailuresentry goes, and with it the map and the known/unknown switch inTestExamplesAnalyseCleanly— it held only this file, so a plainif len(errs) > 0remains.2. The SysML v1 migrator (
7a60fd2de).collection(p)wrotenonuniquefor everyisUnique="false"property andtypeModifier.shape()wroteordered nonuniquefor every[]/[n]MagicDraw type modifier, regardless of the usage the property becomes. For a compositepart/item/action/… whose v2 usage implicitly subsets a unique library feature, that is now invalid notation. Before/after for the fixture inTestCollectionModifiersAreWritten:and the report, where the drop is never silent:
How the decision is made — derived from the semantics, no name table in the migrator:
internal/semantic/semantics/nested.gois refactored so the rule chains run over declaration kinds alone:NestedUsage{Kind, Composite, Portion, Performed, Exhibited, Included, RequirementConstraint}×NestedOwner{Usage | Def}→nestedRuleFQN, with the model-dependent KerML step fallback (subperformances/ownedPerformances/enclosedPerformances, chosen by what the owner conforms to) split off behind astepflag.Model.implicitSubsettingFQNis the same function as before over a symbol; the new exportedImplicitSubsettingCandidates(u, owner) []stringis the kind-only view (one feature, or the three step-fallback features under an occurrence owner).internal/translate/migrate/uniqueness.go:implicitlyUnique(kw, prefix, dir, owner)maps the written keyword (part,item,action, …ref→ a default reference usage, which the parser reads as an attribute) and owner category to those kinds, asks for the candidates, and reads each one's uniqueness from the bundled library throughsemantics.Model.IsUniqueoverlibs.SharedBase()(built once). A directed usage is a parameter and is exempt, asIsParameterexempts it in the checker.migration.writtenUniqueadds the explicit case the fixture itself shows: a usage that redefines or subsets (:>>/:>, incl. the shadow redefinition) a feature that is written unique — declared so in v1, or forced unique by this same rule — must be unique too, else the written:> nfails the same conformance. Cycles settle on what is declared.collection(p, unique),shaped(…, unique)andtypeModifier.shape(unique)take the verdict; association ends, metadata tag attributes, behavior parameters and activity pins passfalse(none of them is a composite nested usage).feature()appends thenonunique is not written: …note, soreport.addmarks the entry approximated per the existing convention.slotConflict(38abe5f12, from review) judges a repeated slot value by the same written uniqueness rather than the v1isUniqueflag: a slot repeating an instance on a composite property that is now written unique is unmapped as a slot conflict, as on any unique feature, and a repeat on a feature an array type modifier writesnonuniqueis admitted.Decision taken, flagged for the maintainer: the modifier is dropped (and recorded) rather than the usage converted to a
ref, since arefwould change composition semantics, and the cascade to an explicit subsetter (babove) follows from the same rule. Say so if you would rather havebhandled differently.Docs:
docs/reference/sysml-v1-migration.mdgains a row forisOrdered/isUnique="false"properties describing the rule and the note, the type-modifier paragraph says[]/[n]writeorderedalone on a usage that must be unique, and the slot-conflict row says "a feature written unique".docs/project/spec-compliance.mdhas no row covering the migrator's uniqueness handling (its uniqueness rows are the value-level and collection ones), so it is untouched;docs/project/validation-constraints.mdalready covers the checker side from #748.Specification basis
SysML v2 1.0 §7.9.3.2 / KerML 1.0 §8.3.3.3.4
Feature::isUniqueand thesubsetting-uniqueness-conformanceconstraint (a subsetting/redefining feature cannot be nonunique if the subsetted/redefined feature is unique); the implicit subsettings of SysML v2 §8.3.x as implemented innested.go. Nospec-compliance.mdrow moves.How it was verified
Tests:
tests/migrate/relations_test.goTestCollectionModifiersAreWritten: fix(check): conformance of an implicit subsetting (#726) #748 changed it to expect the one uniqueness finding atpart n : A[2] nonunique;, pinning the broken output. It now expects the notation above, no diagnostics,_n/_bapproximated with the exact notes, and_o/_u/_e1mapped. The fixture'sois madeisUnique="false"so a legalnonunique(an attribute) stays in the output and the TurtleisNonuniquecheck remains load-bearing.internal/translate/migrate/uniqueness_test.goTestImplicitlyUniqueAgreesWithTheChecker: for every usage keyword the migrator writes ×ref/composite × every owner category, builds<owner> O { <usage> x[*] nonunique; }, skips combinations the owner does not admit at all, and asserts the migrator writesnonuniqueexactly when the checker accepts it. Plus the parameter/ref/attribute exemptions andlibraryUniqueonsubparts(unique) /dataValues(nonunique) / an unknown name.tests/migrate/values_test.goTestPartSlotRepeatingOnAForcedUniquePartIsUnmapped: a nonunique compositesparesis writtenpart spares : MCS[0..*];(approximated) and its slot holding'mcs 2'twice is unmapped with the slot-conflict note.go test -count=1 ./internal/semantic/semantics ./internal/check/...green after thenested.gorefactor (behaviour-preserving;semantics/nested_test.goand the implicit-subsetting conformance goldens unchanged).Baselines, adjudicated:
./scripts/download-pilot-sysml-validator.sh,go run -C tools ./cmd/pilot-diff -update): the only movement is theexamplesroot digest (69a73dac…→4a51ae18…), from the demo edit. Aggregate unchanged: 380 files, 345 fully agreeing, 38 agreed / 40 ours-only / 1616 pilot-only. The baseline records no per-file verdict for the demo (it was not a disagreeing file), so no verdict line moves. Reproduced:rm -rf build/pilot-diff && go run -C tools ./cmd/pilot-diffthendiff <(jq -S . docs/project/pilot-differential-baseline.json) <(jq -S . build/pilot-diff/pilot-diff.json)differs only in the committed"recorded"date line.go test -C tools ./referee/diff -run TestCommittedBaselineStatesThisRepositorysProvenancepasses../scripts/download-pssm-suite.sh,go test -count=1 ./tests/corpus -run TestPSSMSuiteMigration): passes with the ratchet unchanged (14512 approximated / 23544 mapped / 6 skipped / 9660 unmapped / 13 validation-errors) — the suite has no compositeisUnique="false"property that the rule touches. Re-run green after the slot change.go test ./tests/migrate/... ./internal/translate/migrate/...: green, migration goldens unchanged.go test ./...withOPENSYSML_REQUIRE_TRAINING_CORPUS=1 OPENSYSML_REQUIRE_PILOT_CORPORA=1set; no expectation file moves.Definition of done (corpora provisioned per AGENTS.md §2):
Checklist
make testandmake lintpass locallychanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.md(messages-demo-nonunique.fixed.md,migrate-nonunique-composites.fixed.md)make docs-countsrun if a gate count moved (only the pilot-differential examples digest moved; no counted gate moved)F4,K5) in the body, docs, or changelogLink to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/e321310cad9c411eabb8c8aebfc18c4d
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/e321310cad9c411eabb8c8aebfc18c4d?variant=devin
Requested by: @HuiJun