From ac360e7909d068464ea12f9757847ae5e3df4d60 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 03:21:59 +0000 Subject: [PATCH 1/3] fix(record): preserve sequence compatibility with existing definitions Mixed quantity and plain-number sequences now record as text instead of losing a shared unit, and repeated values are rejected against existing unique members while remaining valid for nonunique ones. Co-Authored-By: jason.han --- .../record-sequence-compat.fixed.md | 2 + internal/exec/analysis/record/record.go | 51 +++++++++---- .../analysis/record/sequence_compat_test.go | 73 +++++++++++++++++++ internal/frontend/repl/record.go | 1 + .../repl/record_sequence_compat_test.go | 68 +++++++++++++++++ 5 files changed, 181 insertions(+), 14 deletions(-) create mode 100644 changes/unreleased/record-sequence-compat.fixed.md create mode 100644 internal/exec/analysis/record/sequence_compat_test.go create mode 100644 internal/frontend/repl/record_sequence_compat_test.go diff --git a/changes/unreleased/record-sequence-compat.fixed.md b/changes/unreleased/record-sequence-compat.fixed.md new file mode 100644 index 0000000000..9008afe872 --- /dev/null +++ b/changes/unreleased/record-sequence-compat.fixed.md @@ -0,0 +1,2 @@ +- **Sequence records preserve compatible values.** Mixed quantity and plain-number sequences are recorded as text rather than losing their shared unit, and repeated values refuse an existing unique member while remaining valid for `ordered nonunique` members. +- **Existing sequence declarations retain their uniqueness semantics.** Generated record definitions continue to admit repeated observations, while older unique definitions name `into` when a repeated sequence cannot fit. diff --git a/internal/exec/analysis/record/record.go b/internal/exec/analysis/record/record.go index dea250b7de..b585202bb9 100644 --- a/internal/exec/analysis/record/record.go +++ b/internal/exec/analysis/record/record.go @@ -112,6 +112,9 @@ type Feature struct { // Multi marks a feature whose declared multiplicity admits more than one // value. Multi bool + + // Unique marks a multi-valued feature that holds no two equal values. + Unique bool } // Existing is what Generate must fit the records it makes into. @@ -180,11 +183,12 @@ var reservedFeatures = map[string]bool{ // feature is one member the record definition declares for a run value. type feature struct { - name string - ref bool // object-valued - typ string // declared type as written, "" for a ref - unitOf string // nonempty: this feature is the unit companion of the named one - multi bool // declares [0..*] + name string + ref bool // object-valued + typ string // declared type as written, "" for a ref + unitOf string // nonempty: this feature is the unit companion of the named one + multi bool // declares [0..*] + repeated bool // a sequence contains equal elements } // valueKind classifies how a value is spelled: its declared type and, for a @@ -214,11 +218,12 @@ const ( // whether the member is multi-valued, and — for a quantity — the unit text its // companion feature records. type shape struct { - kind valueKind - literal string - typ string - unit string - multi bool + kind valueKind + literal string + typ string + unit string + multi bool + repeated bool } // classify decides the feature shape a value asks for. @@ -291,7 +296,9 @@ func classifySequence(v runtime.Value, r *Run) shape { case es.kind == settled.kind && es.unit == settled.unit && (es.kind != kindEnum || es.typ == settled.typ): // Same kind; for a quantity es.unit == settled.unit holds the share, and // enumeration literals must spell literals of the one enum. - case numericPair(es.typ, settled.typ): + case (es.kind == kindInteger || es.kind == kindReal) && + (settled.kind == kindInteger || settled.kind == kindReal) && + numericPair(es.typ, settled.typ): settled = shape{kind: kindReal, typ: scalarValuesReal} default: return fallback @@ -299,6 +306,7 @@ func classifySequence(v runtime.Value, r *Run) shape { literals = append(literals, es.literal) } settled.multi = true + settled.repeated = hasRepeatedLiteral(literals) settled.literal = "(" + strings.Join(literals, ", ") + ")" return settled } @@ -466,7 +474,7 @@ func buildFeatures(req *Request) ([]feature, error) { // definition: either way the member settles to Real, an Integer // literal remaining valid under it. if numericPair(cur.typ, sh.typ) { - cur = shape{kind: kindReal, typ: scalarValuesReal, multi: cur.multi} + cur = shape{kind: kindReal, typ: scalarValuesReal, multi: cur.multi, repeated: cur.repeated || sh.repeated} shapes[m.name] = cur continue } @@ -494,8 +502,8 @@ func buildFeatures(req *Request) ([]feature, error) { case c.kind == kindQuantity && (o.kind == kindInteger || o.kind == kindReal): shapes[owner] = c case numericPair(o.typ, c.typ): - shapes[owner] = shape{kind: kindReal, typ: scalarValuesReal, multi: o.multi} - shapes[companion] = shape{kind: kindReal, typ: scalarValuesReal, multi: c.multi} + shapes[owner] = shape{kind: kindReal, typ: scalarValuesReal, multi: o.multi, repeated: o.repeated || c.repeated} + shapes[companion] = shape{kind: kindReal, typ: scalarValuesReal, multi: c.multi, repeated: o.repeated || c.repeated} default: f := feature{name: owner} applyShape(&f, o) @@ -527,6 +535,17 @@ func buildFeatures(req *Request) ([]feature, error) { return feats, nil } +func hasRepeatedLiteral(literals []string) bool { + seen := make(map[string]bool, len(literals)) + for _, literal := range literals { + if seen[literal] { + return true + } + seen[literal] = true + } + return false +} + // numericPair reports whether the types are Integer and Real in either order: // one numeric family for the record definition, settling to Real. func numericPair(a, b string) bool { @@ -538,6 +557,7 @@ func numericPair(a, b string) bool { func applyShape(f *feature, sh shape) { f.ref = sh.kind == kindRef f.multi = sh.multi + f.repeated = sh.repeated switch sh.kind { case kindUnset: f.typ = scalarValuesScalarValue @@ -701,6 +721,9 @@ func checkExisting(req *Request, feats []feature, defName string) error { } return fmt.Errorf("record definition %s declares %s as %s but the run values need %s; record into another package with `into`", def, f.name, kind, want) } + if f.multi && f.repeated && decl.Unique { + return fmt.Errorf("record definition %s declares %s unique but the run values repeat a value; record into another package with `into`", def, f.name) + } if !f.ref && f.typ != "" && decl.TypeFQN != "" && decl.TypeFQN != f.typ && f.typ != scalarValuesScalarValue && decl.TypeFQN != scalarValuesScalarValue { // An Integer literal is valid under a declared Real; the diff --git a/internal/exec/analysis/record/sequence_compat_test.go b/internal/exec/analysis/record/sequence_compat_test.go new file mode 100644 index 0000000000..b8383e277c --- /dev/null +++ b/internal/exec/analysis/record/sequence_compat_test.go @@ -0,0 +1,73 @@ +package record + +import ( + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/exec/runtime" + "github.com/Open-MBEE/OpenSysML/internal/syntax/format" +) + +func TestGenerateSequenceMixedQuantityAndNumberFallsBackToText(t *testing.T) { + res, err := Generate(Request{ + Package: "Records", Case: "P::temps", Provenance: provenance(KindRun), + Runs: []Run{{ + Spell: spell(), + Outputs: []runtime.CalcOutputValue{{Name: "temps", Value: seqOf(kelvin(300), integer(301))}}, + }}, + }) + if err != nil { + t.Fatalf("Generate: %v", err) + } + for _, want := range []string{ + "attribute temps : ScalarValues::String;", + `attribute :>> temps = "[300.0 [K], 301]";`, + } { + if !strings.Contains(res.Source, want) { + t.Errorf("source is missing %q:\n%s", want, res.Source) + } + } + if strings.Contains(res.Source, "tempsUnit") { + t.Errorf("mixed quantity/plain sequence unexpectedly has a unit companion:\n%s", res.Source) + } +} + +func TestGenerateExistingSequenceRejectsRepeatedValuesForUniqueMember(t *testing.T) { + _, err := Generate(Request{ + Package: "Records", Case: "P::temps", Provenance: provenance(KindRun), + Runs: []Run{{Spell: spell(), Outputs: []runtime.CalcOutputValue{{Name: "temps", Value: seqOf(realValue(300), realValue(300), realValue(341.2))}}}}, + Existing: Existing{Definition: true, Stem: "temps", Attributes: map[string]Feature{ + "temps": {TypeFQN: scalarValuesReal, Multi: true, Unique: true}, + }}, + }) + if err == nil || !strings.Contains(err.Error(), "declares temps unique but the run values repeat a value") { + t.Fatalf("Generate = %v, want the unique repeated-value refusal", err) + } +} + +func TestGenerateExistingSequenceCompatibility(t *testing.T) { + cases := map[string]struct { + values []runtime.Value + unique bool + }{ + "unique values in unique member": {values: []runtime.Value{realValue(300), realValue(310)}, unique: true}, + "repeated values in nonunique member": {values: []runtime.Value{realValue(300), realValue(300)}, unique: false}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + res, err := Generate(Request{ + Package: "Records", Case: "P::temps", Provenance: provenance(KindRun), + Runs: []Run{{Spell: spell(), Outputs: []runtime.CalcOutputValue{{Name: "temps", Value: seqOf(tc.values...)}}}}, + Existing: Existing{Definition: true, Stem: "temps", Attributes: map[string]Feature{ + "temps": {TypeFQN: scalarValuesReal, Multi: true, Unique: tc.unique}, + }}, + }) + if err != nil { + t.Fatalf("Generate: %v", err) + } + if _, err := format.Source("", []byte(res.Source), format.DefaultOptions); err != nil { + t.Fatalf("generated source does not parse: %v", err) + } + }) + } +} diff --git a/internal/frontend/repl/record.go b/internal/frontend/repl/record.go index 6405beeee1..1d266643a8 100644 --- a/internal/frontend/repl/record.go +++ b/internal/frontend/repl/record.go @@ -513,6 +513,7 @@ func recordAttributes(idx *symbols.Index, sem *semantics.Model, def *symbols.Sym f.TypeFQN = idx.GetFQN(types[0]) } f.Multi = !sem.GoverningMultiplicityOf(m).AtMostOne() + f.Unique = sem.IsUnique(m) attrs[m.Name] = f } return attrs diff --git a/internal/frontend/repl/record_sequence_compat_test.go b/internal/frontend/repl/record_sequence_compat_test.go new file mode 100644 index 0000000000..e09cf51686 --- /dev/null +++ b/internal/frontend/repl/record_sequence_compat_test.go @@ -0,0 +1,68 @@ +package repl + +import ( + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/semantic/resolve" + "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" +) + +func TestRecordAttributesReportsSequenceUniqueness(t *testing.T) { + for name, declaration := range map[string]string{ + "unique": "attribute temps : Real[0..*];", + "nonunique": "attribute temps : Real[0..*] nonunique;", + } { + t.Run(name, func(t *testing.T) { + s := NewSession() + model := `package Records { + private import ScalarValues::*; + private import AnalysisRecords::*; + part def Rec :> AnalysisRecords::AnalysisRun { ` + declaration + ` } + }` + if errs := errorDiagnostics(s.Submit(model).Diagnostics); len(errs) > 0 { + t.Fatalf("model has errors: %v", errs) + } + idx := s.symbolIndex() + defs := idx.LookupQualified("Records::Rec") + if len(defs) != 1 { + t.Fatalf("Records::Rec resolves to %d symbols", len(defs)) + } + resolver := resolve.New(idx) + sem := semantics.NewModel(resolver) + resolver.SetModel(sem) + feature, ok := recordAttributes(idx, sem, defs[0])["temps"] + if !ok || !feature.Multi { + t.Fatalf("temps = %+v (present %v), want a multi-valued feature", feature, ok) + } + wantUnique := name == "unique" + if feature.Unique != wantUnique { + t.Errorf("temps.Unique = %v, want %v", feature.Unique, wantUnique) + } + }) + } +} + +func TestRecordAttributesFollowsInheritedNonunique(t *testing.T) { + s := NewSession() + model := `package Records { + private import ScalarValues::*; + private import AnalysisRecords::*; + part def Base { attribute temps : Real[0..*] nonunique; } + part def Rec :> Base, AnalysisRecords::AnalysisRun { attribute :>> temps; } + }` + if errs := errorDiagnostics(s.Submit(model).Diagnostics); len(errs) > 0 { + t.Fatalf("model has errors: %v", errs) + } + idx := s.symbolIndex() + defs := idx.LookupQualified("Records::Rec") + if len(defs) != 1 { + t.Fatalf("Records::Rec resolves to %d symbols", len(defs)) + } + resolver := resolve.New(idx) + sem := semantics.NewModel(resolver) + resolver.SetModel(sem) + feature, ok := recordAttributes(idx, sem, defs[0])["temps"] + if !ok || !feature.Multi || feature.Unique { + t.Errorf("temps = %+v (present %v), want inherited multi nonunique feature", feature, ok) + } +} From 2866f312ab1c284443e14bad645e620cde853bf7 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 03:22:51 +0000 Subject: [PATCH 2/3] docs(record): document the repeated-literal check and its changelog fragment Co-Authored-By: jason.han --- changes/unreleased/record-sequence-compat.fixed.md | 4 ++-- internal/exec/analysis/record/record.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/changes/unreleased/record-sequence-compat.fixed.md b/changes/unreleased/record-sequence-compat.fixed.md index 9008afe872..ecf6975a85 100644 --- a/changes/unreleased/record-sequence-compat.fixed.md +++ b/changes/unreleased/record-sequence-compat.fixed.md @@ -1,2 +1,2 @@ -- **Sequence records preserve compatible values.** Mixed quantity and plain-number sequences are recorded as text rather than losing their shared unit, and repeated values refuse an existing unique member while remaining valid for `ordered nonunique` members. -- **Existing sequence declarations retain their uniqueness semantics.** Generated record definitions continue to admit repeated observations, while older unique definitions name `into` when a repeated sequence cannot fit. +- **A sequence mixing a quantity with a plain number records as text.** It has no single unit to record under, so it no longer settles to a bare Real that drops the measured element's unit. +- **A repeated sequence value is refused by an existing unique record member.** Recording into a definition that declares the member `[0..*]` without `nonunique` names the `into` remedy instead of generating a record that fails validation; generated definitions, declared `ordered nonunique`, keep admitting repeats. diff --git a/internal/exec/analysis/record/record.go b/internal/exec/analysis/record/record.go index b585202bb9..708e817410 100644 --- a/internal/exec/analysis/record/record.go +++ b/internal/exec/analysis/record/record.go @@ -535,6 +535,7 @@ func buildFeatures(req *Request) ([]feature, error) { return feats, nil } +// hasRepeatedLiteral reports whether two elements of a sequence spell the same literal. func hasRepeatedLiteral(literals []string) bool { seen := make(map[string]bool, len(literals)) for _, literal := range literals { From 60638609700c0cd5f96efb0be8ffd4ec22a1cba5 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 04:16:08 +0000 Subject: [PATCH 3/3] fix(record): judge repeated sequence elements by value, across runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Element repeats are detected with runtime Set membership — an Integer and the equal Real are one value to a unique member — and a repeat in any run marks the member even when the shape merge settles over runs that did not repeat. Co-Authored-By: jason.han --- internal/exec/analysis/record/record.go | 31 ++++++----- .../analysis/record/sequence_compat_test.go | 53 +++++++++++++++++++ 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/internal/exec/analysis/record/record.go b/internal/exec/analysis/record/record.go index 708e817410..80fa833510 100644 --- a/internal/exec/analysis/record/record.go +++ b/internal/exec/analysis/record/record.go @@ -274,7 +274,6 @@ func classify(v runtime.Value, r *Run) shape { // beside its kind, so the whole value falls back to its text. The empty sequence // is unset but multi-valued: it settles a member to [0..*] and spells `()`. func classifySequence(v runtime.Value, r *Run) shape { - fallback := shape{kind: kindString, typ: scalarValuesString, literal: source.StringText(spellText(v, r))} seq := v.Sequence() var elements []runtime.Value if seq != nil { @@ -283,6 +282,8 @@ func classifySequence(v runtime.Value, r *Run) shape { if len(elements) == 0 { return shape{kind: kindUnset, multi: true, literal: "()"} } + repeated := hasRepeatedElement(elements) + fallback := shape{kind: kindString, typ: scalarValuesString, literal: source.StringText(spellText(v, r)), repeated: repeated} literals := make([]string, 0, len(elements)) var settled shape for i, element := range elements { @@ -306,7 +307,7 @@ func classifySequence(v runtime.Value, r *Run) shape { literals = append(literals, es.literal) } settled.multi = true - settled.repeated = hasRepeatedLiteral(literals) + settled.repeated = repeated settled.literal = "(" + strings.Join(literals, ", ") + ")" return settled } @@ -418,6 +419,9 @@ func buildFeatures(req *Request) ([]feature, error) { // First pass: settle each member's shape over every run that supplies it. var names []string shapes := map[string]shape{} + // A repeated element anywhere in a member's runs marks its feature, even + // when a merge settles the shape over values that did not repeat. + repeated := map[string]bool{} for i := range req.Runs { for _, m := range members(req.Runs[i]) { if m.inOf == "" { @@ -429,6 +433,9 @@ func buildFeatures(req *Request) ([]feature, error) { return nil, fmt.Errorf("case %s: parameter %q shares a name with a feature of AnalysisRecords::AnalysisRun", req.Case, m.name) } sh := classify(m.value, &req.Runs[i]) + if sh.repeated { + repeated[m.name] = true + } cur, seen := shapes[m.name] if !seen { names = append(names, m.name) @@ -474,7 +481,7 @@ func buildFeatures(req *Request) ([]feature, error) { // definition: either way the member settles to Real, an Integer // literal remaining valid under it. if numericPair(cur.typ, sh.typ) { - cur = shape{kind: kindReal, typ: scalarValuesReal, multi: cur.multi, repeated: cur.repeated || sh.repeated} + cur = shape{kind: kindReal, typ: scalarValuesReal, multi: cur.multi} shapes[m.name] = cur continue } @@ -502,8 +509,8 @@ func buildFeatures(req *Request) ([]feature, error) { case c.kind == kindQuantity && (o.kind == kindInteger || o.kind == kindReal): shapes[owner] = c case numericPair(o.typ, c.typ): - shapes[owner] = shape{kind: kindReal, typ: scalarValuesReal, multi: o.multi, repeated: o.repeated || c.repeated} - shapes[companion] = shape{kind: kindReal, typ: scalarValuesReal, multi: c.multi, repeated: o.repeated || c.repeated} + shapes[owner] = shape{kind: kindReal, typ: scalarValuesReal, multi: o.multi} + shapes[companion] = shape{kind: kindReal, typ: scalarValuesReal, multi: c.multi} default: f := feature{name: owner} applyShape(&f, o) @@ -527,6 +534,7 @@ func buildFeatures(req *Request) ([]feature, error) { } f := feature{name: name} applyShape(&f, shapes[name]) + f.repeated = repeated[name] feats = append(feats, f) if shapes[name].kind == kindQuantity { feats = append(feats, feature{name: name + "Unit", typ: scalarValuesString, unitOf: name}) @@ -535,14 +543,14 @@ func buildFeatures(req *Request) ([]feature, error) { return feats, nil } -// hasRepeatedLiteral reports whether two elements of a sequence spell the same literal. -func hasRepeatedLiteral(literals []string) bool { - seen := make(map[string]bool, len(literals)) - for _, literal := range literals { - if seen[literal] { +// hasRepeatedElement reports whether two elements of a sequence are equal values. +func hasRepeatedElement(elements []runtime.Value) bool { + seen := runtime.NewSet() + for _, element := range elements { + if seen.Contains(element) { return true } - seen[literal] = true + seen.Add(element) } return false } @@ -558,7 +566,6 @@ func numericPair(a, b string) bool { func applyShape(f *feature, sh shape) { f.ref = sh.kind == kindRef f.multi = sh.multi - f.repeated = sh.repeated switch sh.kind { case kindUnset: f.typ = scalarValuesScalarValue diff --git a/internal/exec/analysis/record/sequence_compat_test.go b/internal/exec/analysis/record/sequence_compat_test.go index b8383e277c..d7c0f865c6 100644 --- a/internal/exec/analysis/record/sequence_compat_test.go +++ b/internal/exec/analysis/record/sequence_compat_test.go @@ -45,6 +45,59 @@ func TestGenerateExistingSequenceRejectsRepeatedValuesForUniqueMember(t *testing } } +// Value equality, not literal spelling, detects a repeat: an Integer 1 and a +// Real 1.0 are one value to a unique member. +func TestGenerateExistingSequenceRejectsARepeatAcrossNumericKinds(t *testing.T) { + _, err := Generate(Request{ + Package: "Records", Case: "P::temps", Provenance: provenance(KindRun), + Runs: []Run{{Spell: spell(), Outputs: []runtime.CalcOutputValue{{Name: "temps", Value: seqOf(integer(1), realValue(1.0))}}}}, + Existing: Existing{Definition: true, Stem: "temps", Attributes: map[string]Feature{ + "temps": {TypeFQN: scalarValuesReal, Multi: true, Unique: true}, + }}, + }) + if err == nil || !strings.Contains(err.Error(), "declares temps unique but the run values repeat a value") { + t.Fatalf("Generate = %v, want the unique repeated-value refusal", err) + } +} + +// A repeat in a later run marks the member even though the merge settles its +// shape over runs that did not repeat. +func TestGenerateExistingSequenceARepeatInALaterRun(t *testing.T) { + cases := map[string]struct { + unique bool + wantErr bool + }{ + "unique member refuses": {unique: true, wantErr: true}, + "nonunique member accepts": {unique: false}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + res, err := Generate(Request{ + Package: "Records", Case: "P::temps", Provenance: provenance(KindRun), + Runs: []Run{ + {Spell: spell(), Outputs: []runtime.CalcOutputValue{{Name: "temps", Value: seqOf(realValue(300), realValue(310))}}}, + {Spell: spell(), Outputs: []runtime.CalcOutputValue{{Name: "temps", Value: seqOf(realValue(300), realValue(300))}}}, + }, + Existing: Existing{Definition: true, Stem: "temps", Attributes: map[string]Feature{ + "temps": {TypeFQN: scalarValuesReal, Multi: true, Unique: tc.unique}, + }}, + }) + if tc.wantErr { + if err == nil || !strings.Contains(err.Error(), "declares temps unique but the run values repeat a value") { + t.Fatalf("Generate = %v, want the unique repeated-value refusal", err) + } + return + } + if err != nil { + t.Fatalf("Generate: %v", err) + } + if _, err := format.Source("", []byte(res.Source), format.DefaultOptions); err != nil { + t.Fatalf("generated source does not parse: %v", err) + } + }) + } +} + func TestGenerateExistingSequenceCompatibility(t *testing.T) { cases := map[string]struct { values []runtime.Value