Skip to content

fix(check): conformance of an implicit subsetting (#726) - #748

Merged
HuiJun merged 4 commits into
Open-MBEE:developfrom
someshSandbox:fix/726-implicit-subsetting-uniqueness
Oct 1, 2026
Merged

HuiJun merged 4 commits into
Open-MBEE:developfrom
someshSandbox:fix/726-implicit-subsetting-uniqueness

Conversation

@someshSandbox

@someshSandbox someshSandbox commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #726.

validateSubsettingUniquenessConformance is a constraint on a Subsetting, and an implied Subsetting is a Subsetting too. Model.ConformanceViolations checked only the written :>/:>> relationships, so a feature that implicitly subsets a unique library feature could be declared nonunique unnoticed.

model develop this PR
action def A { action b[*] nonunique; } (implicitly subsets Actions::Action::subactions, Action[0..*], unique) no errors 3:9: error: Subsetting/redefining feature cannot be nonunique if subsetted/redefined feature is unique
action def A { action b[*] nonunique :> subactions; } that error at :> subactions the same, once
part def A { part b[*] nonunique; } (implicitly subsets Items::Item::subitems, Item[0..*], unique) no errors the same error at the declaration
abstract flow def F; part def C { message m : F[*] nonunique; } (a message is an action; in a part it implicitly subsets Parts::Part::ownedActions, Action[0..*], unique) no errors the same error at the declaration
action def A { action b[*]; }, part def A { ref part b[*] nonunique; }, part def A { attribute b[*] nonunique; } no errors no errors

Change:

  • ConformanceViolations also runs the restriction checks (uniqueness, and constancy with it) against each feature ImplicitSubsettings gives the usage. The finding is at the declaration, since nothing is written to point at.
  • A feature the usage already subsets or redefines explicitly is checked once, at the written reference.
  • validation-constraints.md records the implicit case.
  • tests/migrate: a SysML v1 composite property with isUnique="false" still migrates as written, to part n : A[2] nonunique; in a part. That part implicitly subsets Item::subitems, so TestCollectionModifiersAreWritten now expects that one uniqueness finding at n, where it expected none.
  • examples/parser_features_demo_messages_events.sysml declares its two messages in MessageChannel nonunique, which the constraint forbids, since a message in a part implicitly subsets the unique Part::ownedActions. The example is left as it is, so the pilot-differential baseline's examples digest doesn't move, and it is listed in examplesKnownFailures with that reason. Fixing the example and re-recording the baseline is Example parser_features_demo_messages_events.sysml declares nonunique messages that subset a unique feature #758.

Tests:

  • passes/w8b_redefinition_conformance_test.go:TestW8BUniquenessConformanceOfAnImplicitSubsetting covers every row above.
  • internal/semantic/..., internal/check/..., internal/exec/runtime/..., internal/translate/..., tests/corpus/..., tests/export/..., tests/identity/..., internal/frontend/grpc/..., internal/workspace/... and cmd/sysml/... pass. The bundled library raises no new finding, and the one example that does is listed above.

🤖 Generated with Claude Code

A Subsetting a feature has implicitly is a Subsetting all the same:
uniqueness and constancy conformance now hold of the feature a usage
implicitly subsets (a composite action in an action, Action::subactions;
a composite part in a part, Item::subitems; a message in a part,
Part::ownedActions), reported at the declaration and once where the feature
also subsets it explicitly. The one example that broke the constraint drops
its nonunique.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

someshSandbox and others added 3 commits September 30, 2026 08:43
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A v1 composite property with isUnique=false migrates as written, to a
composite part that implicitly subsets the unique Item::subitems; the
test now expects that one finding, at n, instead of none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The demo's two nonunique messages break subsetting uniqueness conformance.
Rather than change the example (and move the pilot-differential baseline's
examples digest), it is restored and listed in examplesKnownFailures with
that reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ Shipped messages demo fails validation

Both nonunique messages implicitly subset the unique Part::ownedActions, so -validate rejects the shipped demo. The examplesKnownFailures entry only suppresses the test failure.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

@HuiJun

HuiJun commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

This one took a bit of review, especially when this points out a gap in the pilot implementation. After review, I think I agree with you on this, though #758 would only fix the test. We'll have to update some migration mapping as well. Thanks Somesh!

@HuiJun
HuiJun merged commit 5fb9a84 into Open-MBEE:develop Oct 1, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subsetting uniqueness conformance is not checked for an implicit subsetting

2 participants