fix(record): keep mixed-unit sequences as text and refuse repeats into a unique member

changes/unreleased/record-sequence-compat.fixed.md

@@ -0,0 +1,2 @@
+- **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.

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.
@@ -269,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 {
@@ -278,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 {
@@ -291,14 +297,17 @@ 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
 		}
 		literals = append(literals, es.literal)
 	}
 	settled.multi = true
+	settled.repeated = repeated
 	settled.literal = "(" + strings.Join(literals, ", ") + ")"
 	return settled
 }
@@ -410,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 == "" {
@@ -421,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)
@@ -519,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})
@@ -527,6 +543,18 @@ func buildFeatures(req *Request) ([]feature, error) {
 	return feats, nil
 }
 
+// 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.Add(element)
+	}
+	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 {
@@ -701,6 +729,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

internal/exec/analysis/record/sequence_compat_test.go

@@ -0,0 +1,126 @@
+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)
+	}
+}
+
+// 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("<record>", []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
+		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("<record>", []byte(res.Source), format.DefaultOptions); err != nil {
+				t.Fatalf("generated source does not parse: %v", err)
+			}
+		})
+	}
+}

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

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)
+	}
+}